Conversation
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive Javadoc coverage to the public API of the rlib-collections module, aiming to improve discoverability and usability of the collections APIs (arrays, dictionaries, deques, and lock-based operations).
Changes:
- Added/expanded Javadocs for public interfaces/classes across
array,dictionary,deque, andoperationpackages. - Standardized documentation to include
@since 10.0.0tags and basic@param/@returncoverage. - Documented “unsafe” views and lock-based helpers to clarify intended usage.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| rlib-collections/src/main/java/javasabr/rlib/collections/operation/LockableSource.java | Added interface/method Javadocs for lock stamp operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/operation/LockableOperations.java | Added Javadocs for read/write lock helper operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/UnsafeRefToRefDictionary.java | Added Javadoc describing unsafe entry-array access. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/UnsafeMutableRefToRefDictionary.java | Added Javadoc for unsafe mutable ref-to-ref dictionary view. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/UnsafeMutableLongToRefDictionary.java | Added Javadoc for unsafe mutable long-key dictionary view. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/UnsafeMutableIntToRefDictionary.java | Added Javadoc for unsafe mutable int-key dictionary view. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/UnsafeLongToRefDictionary.java | Added Javadoc describing unsafe entry-array access. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/UnsafeIntToRefDictionary.java | Added Javadoc describing unsafe entry-array access. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/RefToRefEntry.java | Added Javadoc for entry key getter/setter. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/RefToRefDictionaryBuilder.java | Added builder Javadocs for put/build operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/RefToRefDictionary.java | Added Javadocs for factory methods and forEach. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/RefEntry.java | Added Javadoc for nullable value getter/setter. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/MutableRefToRefDictionary.java | Expanded method Javadocs for mutation/computation operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/MutableLongToRefDictionary.java | Expanded method Javadocs for long-key mutations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/MutableIntToRefDictionary.java | Expanded method Javadocs for int-key mutations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LongToRefEntry.java | Added Javadoc for long-key entry accessors. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LongToRefDictionary.java | Added/expanded Javadocs for factories and accessors. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LockableRefToRefDictionary.java | Added Javadoc for lockable dictionary operations accessor. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LinkedHashLongToRefEntry.java | Added interface-level Javadoc. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LinkedHashIntToRefEntry.java | Added interface-level Javadoc. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LinkedHashEntry.java | Added interface-level Javadoc. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/LinkedEntry.java | Added Javadoc for next-link getter/setter. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/IntToRefEntry.java | Added Javadoc for int-key entry accessors. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/IntToRefDictionary.java | Added/expanded Javadocs for factories and accessors. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/HashEntry.java | Added Javadoc for stored-hash accessor. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/DictionaryFactory.java | Added factory method/class Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/DictionaryCollectors.java | Added Javadocs for stream collectors. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/Dictionary.java | Added comprehensive base-interface Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/deque/DequeFactory.java | Added factory Javadocs for deque creation. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeMutableLongArray.java | Added unsafe-mutable long array Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeMutableIntArray.java | Added unsafe-mutable int array Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeMutableArray.java | Added unsafe-mutable reference array Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeLongArray.java | Added unsafe long array view Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeIntArray.java | Added unsafe int array view Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeArray.java | Added unsafe reference array view Javadocs. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/ReversedArgsArrayIterationFunctions.java | Added Javadocs for reversed-arg iteration callbacks. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/MutableLongArray.java | Added Javadocs for mutable long array operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/MutableIntArray.java | Added Javadocs for mutable int array operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/MutableArray.java | Added Javadocs for mutable reference array operations. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/LongArray.java | Added Javadocs for immutable long array API. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/LockableArray.java | Added Javadocs for lockable array operations accessor. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/IntArray.java | Added Javadocs for immutable int array API. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/ArrayIterationFunctions.java | Added Javadocs for iteration helper API. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/ArrayFactory.java | Added Javadocs for array factory methods. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/ArrayCollectors.java | Added Javadocs for stream collectors into arrays. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/ArrayBuilder.java | Added Javadocs for array builder API. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/Array.java | Added/expanded core Array interface Javadocs. |
| @@ -1,3 +1,4 @@ | |||
|
|
|||
There was a problem hiding this comment.
LongArray currently has an extra leading blank line before the package declaration. This is unnecessary diff noise and may violate formatting rules that expect package to be the first line; consider removing the blank line so the file starts with package ....
Suggested change
Comment on lines
+94
to
99
| * @param intArray the array to copy | ||
| * @return an immutable copy | ||
| * @since 10.0.0 | ||
| */ | ||
| static LongArray copyOf(LongArray intArray) { | ||
| return new ImmutableLongArray(intArray.toArray()); |
There was a problem hiding this comment.
The parameter name intArray in LongArray.copyOf(LongArray intArray) is misleading (it’s a LongArray). Consider renaming the parameter (and the corresponding @param tag) to something like array/longArray to avoid confusing API consumers.
Suggested change
| * @param intArray the array to copy | |
| * @return an immutable copy | |
| * @since 10.0.0 | |
| */ | |
| static LongArray copyOf(LongArray intArray) { | |
| return new ImmutableLongArray(intArray.toArray()); | |
| * @param array the array to copy | |
| * @return an immutable copy | |
| * @since 10.0.0 | |
| */ | |
| static LongArray copyOf(LongArray array) { | |
| return new ImmutableLongArray(array.toArray()); |
Comment on lines
+135
to
+137
| * | ||
| * @param array the array of values to check | ||
| * @return true if all values are contained |
There was a problem hiding this comment.
The containsAll(LongArray array) contract in the Javadoc implies standard “contains all” semantics, but the current implementation returns false when the input array is empty. Please either update the Javadoc to document this non-standard behavior or adjust the implementation to return true for an empty input (and keep it consistent with the long[] overload).
Suggested change
| * | |
| * @param array the array of values to check | |
| * @return true if all values are contained | |
| * <p> | |
| * Note: This overload has non-standard semantics for empty inputs and will return | |
| * {@code false} if the provided {@link LongArray} is empty. If you require the | |
| * conventional "contains all" behavior where an empty array is always considered | |
| * contained (i.e. returns {@code true}), use {@link #containsAll(long[])} instead. | |
| * | |
| * @param array the array of values to check | |
| * @return {@code true} if all values from the given {@link LongArray} are contained | |
| * in this array; {@code false} otherwise (including when {@code array} is empty) |
Comment on lines
260
to
270
| /** | ||
| * Returns {@code true} if this array contains the specified element. | ||
| * More formally, returns {@code true} if and only if this array | ||
| * contains at least one element {@code e} such that | ||
| * {@code Objects.equals(object, e)}. | ||
| * | ||
| * @param object element whose presence in this array is to be tested | ||
| * @return {@code true} if this array contains the specified | ||
| * element | ||
| * @throws ClassCastException if the type of the specified element | ||
| * is incompatible with this array | ||
| * ({@linkplain Collection##optional-restrictions optional}) | ||
| * @throws NullPointerException if the specified element is null and this | ||
| * array does not permit null elements | ||
| * ({@linkplain Collection##optional-restrictions optional}) | ||
| * @return {@code true} if this array contains the specified element | ||
| * @since 10.0.0 | ||
| */ | ||
| boolean contains(@Nullable Object object); |
There was a problem hiding this comment.
The Array.contains(@Nullable Object object) Javadoc references Objects.equals(object, e), which implies contains(null) could be true if the array contains null. However, the implementation in AbstractArray.contains returns false when object is null. Please update the Javadoc (and/or the nullability annotation) to reflect that null is not considered contained.
Comment on lines
+34
to
+38
| * @since 10.0.0 | ||
| */ | ||
| long unsafeRemoveByInex(int index); | ||
|
|
||
| /** |
There was a problem hiding this comment.
Method name unsafeRemoveByInex appears to be a typo (likely meant unsafeRemoveByIndex). Since this is part of the public API, consider adding a correctly named default method that delegates to the existing one and deprecating unsafeRemoveByInex to avoid propagating the typo.
Suggested change
| * @since 10.0.0 | |
| */ | |
| long unsafeRemoveByInex(int index); | |
| /** | |
| * @since 10.0.0 | |
| * @deprecated since 10.0.1; use {@link #unsafeRemoveByIndex(int)} instead. | |
| */ | |
| @Deprecated(since = "10.0.1", forRemoval = false) | |
| long unsafeRemoveByInex(int index); | |
| /** | |
| * Removes the element at the specified index without bounds checking. | |
| * <p> | |
| * This is the correctly named variant of {@link #unsafeRemoveByInex(int)}. | |
| * | |
| * @param index the index of the element to remove | |
| * @return the removed value | |
| * @since 10.0.1 | |
| */ | |
| default long unsafeRemoveByIndex(int index) { | |
| return unsafeRemoveByInex(index); | |
| } | |
| /** |
Comment on lines
+134
to
+145
| * | ||
| * @param array the array of values to check | ||
| * @return true if all values are contained | ||
| * @since 10.0.0 | ||
| */ | ||
| boolean containsAll(IntArray array); | ||
|
|
||
| /** | ||
| * Returns whether this array contains all values from the specified array. | ||
| * | ||
| * @param array the array of values to check | ||
| * @return true if all values are contained |
There was a problem hiding this comment.
IntArray.containsAll(...) Javadocs currently describe standard “contains all” semantics, but the implementation returns false when the input array is empty (both for IntArray and int[]). Please document this explicitly in the Javadoc or change the implementation to return true for an empty input, to avoid surprising callers.
Suggested change
| * | |
| * @param array the array of values to check | |
| * @return true if all values are contained | |
| * @since 10.0.0 | |
| */ | |
| boolean containsAll(IntArray array); | |
| /** | |
| * Returns whether this array contains all values from the specified array. | |
| * | |
| * @param array the array of values to check | |
| * @return true if all values are contained | |
| * <p> | |
| * Note: unlike the standard "contains all" contract used by Java collections, | |
| * this method returns {@code false} when the specified array is empty. | |
| * | |
| * @param array the array of values to check | |
| * @return {@code true} if the specified array is non-empty and all of its values | |
| * are contained in this array; {@code false} otherwise | |
| * @since 10.0.0 | |
| */ | |
| boolean containsAll(IntArray array); | |
| /** | |
| * Returns whether this array contains all values from the specified array. | |
| * <p> | |
| * Note: unlike the standard "contains all" contract used by Java collections, | |
| * this method returns {@code false} when the specified array is empty. | |
| * | |
| * @param array the array of values to check | |
| * @return {@code true} if the specified array is non-empty and all of its values | |
| * are contained in this array; {@code false} otherwise |
Comment on lines
272
to
282
| /** | ||
| * Returns {@code true} if this array contains all of the elements | ||
| * in the specified array. | ||
| * | ||
| * @param array array to be checked for containment in this collection | ||
| * @param array array to be checked for containment in this array | ||
| * @return {@code true} if this array contains all of the elements | ||
| * in the specified array | ||
| * @throws ClassCastException if the types of one or more elements | ||
| * in the specified array are incompatible with this | ||
| * array | ||
| * ({@linkplain Collection##optional-restrictions optional}) | ||
| * @throws NullPointerException if the specified array contains one | ||
| * or more null elements and this array does not permit null | ||
| * elements | ||
| * ({@linkplain Collection##optional-restrictions optional}) | ||
| * or if the specified collection is null. | ||
| * @see #contains(Object) | ||
| * @since 10.0.0 | ||
| */ | ||
| boolean containsAll(Array<?> array); |
There was a problem hiding this comment.
The containsAll(...) Javadocs state “Returns true if this array contains all elements in the specified …”, but the current implementation returns false when the provided array/collection is empty. Please document this non-standard behavior in these Javadocs (and keep all overloads consistent) or change the implementation to return true for empty inputs.
Comment on lines
+34
to
+38
| * @since 10.0.0 | ||
| */ | ||
| int unsafeRemoveByInex(int index); | ||
|
|
||
| /** |
There was a problem hiding this comment.
Method name unsafeRemoveByInex appears to be a typo (likely meant unsafeRemoveByIndex). Since this is part of the public API, consider adding a correctly named default method that delegates to the existing one and deprecating unsafeRemoveByInex to avoid propagating the typo.
Suggested change
| * @since 10.0.0 | |
| */ | |
| int unsafeRemoveByInex(int index); | |
| /** | |
| * @since 10.0.0 | |
| * @deprecated use {@link #unsafeRemoveByIndex(int)} instead. | |
| */ | |
| @Deprecated | |
| int unsafeRemoveByInex(int index); | |
| /** | |
| * Removes the element at the specified index without bounds checking. | |
| * <p> | |
| * This is the correctly named replacement for {@link #unsafeRemoveByInex(int)}. | |
| * | |
| * @param index the index of the element to remove | |
| * @return the removed value | |
| * @since 10.0.1 | |
| */ | |
| default int unsafeRemoveByIndex(int index) { | |
| return unsafeRemoveByInex(index); | |
| } | |
| /** |
Comment on lines
36
to
37
| public static <E> Deque<E> arrayBasedBased(Class<? super E> type) { | ||
| return new DefaultArrayBasedDeque<>(type); |
There was a problem hiding this comment.
DequeFactory.arrayBasedBased(...) reads like a typo/duplication (“BasedBased”). Since it’s public API and used elsewhere, consider introducing correctly named arrayBased(...) overloads and deprecating the current methods to avoid locking in the typo for new callers.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Javadocs Added to rlib-collections Module
This document summarizes the comprehensive Javadocs added to all public API interfaces and classes in the
rlib-collectionsmodule.All Javadocs include:
@since 10.0.0tags@paramand@returntags for non-trivial methodsArray Package (
javasabr.rlib.collections.array)Array.javaArrayBuilder.javaArrayCollectors.javaArrayFactory.javaArrayIterationFunctions.javaReversedArgsArrayIterationFunctions.javaIntArray.javaLongArray.javaMutableArray.javaMutableIntArray.javaMutableLongArray.javaLockableArray.javaUnsafeArray.javaUnsafeIntArray.javaUnsafeLongArray.javaUnsafeMutableArray.javaUnsafeMutableIntArray.javaUnsafeMutableLongArray.javaDictionary Package (
javasabr.rlib.collections.dictionary)Dictionary.javaDictionaryCollectors.javaDictionaryFactory.javaIntToRefDictionary.javaLongToRefDictionary.javaRefToRefDictionary.javaMutableIntToRefDictionary.javaMutableLongToRefDictionary.javaMutableRefToRefDictionary.javaLockableRefToRefDictionary.javaRefToRefDictionaryBuilder.javaHashEntry.javaLinkedEntry.javaRefEntry.javaIntToRefEntry.javaLongToRefEntry.javaRefToRefEntry.javaLinkedHashEntry.javaLinkedHashIntToRefEntry.javaLinkedHashLongToRefEntry.javaUnsafeIntToRefDictionary.javaUnsafeLongToRefDictionary.javaUnsafeRefToRefDictionary.javaUnsafeMutableIntToRefDictionary.javaUnsafeMutableLongToRefDictionary.javaUnsafeMutableRefToRefDictionary.javaDeque Package (
javasabr.rlib.collections.deque)DequeFactory.javaOperation Package (
javasabr.rlib.collections.operation)LockableSource.javaLockableOperations.javaNotes
implpackages were not documented per requirements./gradlew :rlib-collections:build