Introduce ImmutableListFactory.ofNCopiesOf method with implementation
#1168
Conversation
|
Also I'm not quite sure where/how to add unit tests... this project is surprisingly complex. Any pointers are appreciated. |
Partially addresses eclipse#1166; does not include primitive support yet
Hi @dmlloyd - Apologies for the late response. I took a quick look at your PR to help you with this question. For the changes, you are making in For the changes you are making on the
Hope this helps. Please reach back, if you have any more questions. Thank you for your contribution! |
| /** | ||
| * This is a single element immutable List which is created by calling | ||
| * Immutable.newListWith(one) method. | ||
| */ |
There was a problem hiding this comment.
This Javadoc looks pasted from a singleton list.
| /** | ||
| * @since 12.0 | ||
| */ | ||
| <T> ImmutableList<T> ofNCopiesOf(int copies, T item); |
There was a problem hiding this comment.
We have a similar method MutableListFactory#withNValues(). I think it's awkward to have "of" in the name twice. I'm also not sure if "copies" is appropriate in the name when these are all the same item, but then again, that's true of java.util.Collections#nCopies where the name comes from.
Right now I'm leaning toward also using withNValues as the name but I'd like to hear opinions.
|
|
||
| ImmutableNCopiesList(T item, int copies) | ||
| { | ||
| assert copies > 1; |
There was a problem hiding this comment.
The assertion is fine, but we shouldn't use the assert keyword.
| public int indexOf(Object object) | ||
| { | ||
| return Objects.equals(this.item, object) ? 0 : -1; | ||
| } |
There was a problem hiding this comment.
Could delegate to contains() here
| public ImmutableList<T> select(Predicate<? super T> predicate) | ||
| { | ||
| return predicate.accept(this.item) ? this : Lists.immutable.empty(); | ||
| } |
There was a problem hiding this comment.
It's odd to see select() implemented without reject() right next to it.
Partially addresses #1166; does not include primitive support yet