felipecrv commented on code in PR #35003:
URL: https://github.com/apache/arrow/pull/35003#discussion_r1370830276
##########
cpp/src/arrow/array/diff.cc:
##########
@@ -164,56 +382,38 @@ class QuadraticSpaceMyersDiff {
: base_(base),
target_(target),
pool_(pool),
- value_comparator_(GetValueComparator(*base.type())),
- base_begin_(0),
base_end_(base.length()),
- target_begin_(0),
target_end_(target.length()),
- endpoint_base_({ExtendFrom({base_begin_, target_begin_}).base}),
insert_({true}) {
- if ((base_end_ - base_begin_ == target_end_ - target_begin_) &&
- endpoint_base_[0] == base_end_) {
- // trivial case: base == target
- finish_index_ = 0;
- }
- }
-
- bool ValuesEqual(int64_t base_index, int64_t target_index) const {
- bool base_null = base_.IsNull(base_index);
- bool target_null = target_.IsNull(target_index);
- if (base_null || target_null) {
- // If only one is null, then this is false, otherwise true
- return base_null && target_null;
- }
- return value_comparator_(base_, base_index, target_, target_index);
+ // endpoint_base_ is initialized when Diff() is called to start the
algorithm.
Review Comment:
That is how it was before, but in [diff.cc: Remove the unsafe creation of
value
comparators](https://github.com/apache/arrow/pull/35003/commits/cdbe50ec07699aa4f267e3061e7186c979aadcbe)
I refactored to make it possible to do proper error handling of the comparator
creation. The comparator is created on `Diff` (allowed to return a `Status`
unlike a class constructor) and then injected to all the other member functions
as an argument. To make it more explicit that `Diff` is the only valid entry
point into the class, I've made all the other functions `private` except for
`Diff`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
