dcapwell commented on code in PR #106: URL: https://github.com/apache/cassandra-accord/pull/106#discussion_r1698934068
########## accord-core/src/main/java/accord/utils/MergeFewDisjointSortedListsCursor.java: ########## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package accord.utils; + +import java.util.Arrays; +import javax.annotation.Nonnull; + +public class MergeFewDisjointSortedListsCursor<T extends Comparable<? super T>> implements SortedCursor<T> +{ + // Holds and is comparable by the head item of an iterator it owns + protected static final class Candidate<T extends Comparable<? super T>> implements Comparable<Candidate<T>> + { + private final SortedList<? extends T> list; + private int itemIdx; + private T item; + + public Candidate(@Nonnull SortedList<? extends T> list) + { + Invariants.checkState(!list.isEmpty()); + this.list = list; + this.item = list.get(0); Review Comment: the `SortedList` interface does not block `null`, and `null` will trick this class to think the cursor is fully consumed. * `SortedArrayList` and `ExtendedSortedArrayList` will NPE as it doesn't allow overriding the `Comparator` * `SortedRelationList` will accept `null` but most methods that touch the data will NPE. This is public and only using in the Deps classes, so current usage doesn't have `null` (things can always change) if `SortedList` documented that `null` is not allowed and `SortedRelationList` validates its input, then the lack of `null` check here is fine. The main concern is not the code as it is today, but how things could evolve in the future... things get reused and assumptions change, so its good to document and/or enforce assumptions early on. ########## accord-core/src/main/java/accord/primitives/Txn.java: ########## @@ -397,7 +397,7 @@ default Writes execute(TxnId txnId, Timestamp executeAt, @Nullable Data data) default AsyncChain<Data> read(SafeCommandStore safeStore, Timestamp executeAt, Ranges unavailable) { Ranges ranges = safeStore.ranges().allAt(executeAt).subtract(unavailable); - List<AsyncChain<Data>> chains = Routables.foldlMinimal(keys(), ranges, (key, accumulate, index) -> { + List<AsyncChain<Data>> chains = Routables.foldlMinimal(keys(), ranges, (key, accumulate, index) -> { Review Comment: style issue... fixed in my branch -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
