You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A complicated but hopefully minor issue was discovered during a workshop for Grace Hopper Celebration. A new contributor upgraded the Eclipse Collections Kata to version 12.0.0.M3 and a test began failing in a class called MultiReaderCollectionsTest. The methods select, selectWith, reject, rejectWith were were pulled up in the following PR.
These methods are now returning a MultiReaderFastList instead of a FastList. From an interface perspective, this is fine. Unfortunately, there is a behavior change because MultiReaderList implementations do not allow iterator to be used, so methods like equals will not work without taking an explicit lock.
The simplest solution would be to return the methods to their previous state. An alternative solution would be to make sure there are covariant overrides for select, selectWith, reject, rejectWith that return MultiReaderList.
This is the failing test code in the Eclipse Collections Kata using 12.0.0.M3. The test was disabled as a short term resolution
This is the modified version of the code I had to write in order to get the test to pass. Note the explicit casts that were required due to the lack of covariant overrides.
@Test
@Tag("SOLUTION")
public void multiReaderListFiltering()
{
MultiReaderList<Integer> list =
Lists.multiReader.with(1, 2, 3, 4, 5);
// equals has a hidden iterator so use read lock
list.withReadLockAndDelegate(delegate ->
Assertions.assertEquals(Interval.oneTo(5), delegate));
Predicate<Integer> isEven = each -> each % 2 == 0;
// TODO: MultiReaderFastList is now returning a MultiReaderFastList for select and reject
// but is not providing a covariant override of the method so a cast is required
MultiReaderList<Integer> evens = (MultiReaderList<Integer>) list.select(isEven);
MultiReaderList<Integer> odds = (MultiReaderList<Integer>) list.reject(isEven);
PartitionList<Integer> partition = list.partition(isEven);
// equals has a hidden iterator so use read lock
evens.withReadLockAndDelegate(delegate -> Assertions.assertEquals(List.of(2, 4), delegate));
odds.withReadLockAndDelegate(delegate -> Assertions.assertEquals(List.of(1, 3, 5), delegate));
Assertions.assertEquals(evens, partition.getSelected());
Assertions.assertEquals(odds, partition.getRejected());
}
I think these four methods should be reverted back in the short-term, because they are now inconsistent with all of the collect methods on MultiReaderFastList which return the same type as a delegate, which would be FastList.
Thoughts?
The text was updated successfully, but these errors were encountered:
The select/reject methods usually return the same type, filtered down. The collect methods usually return a different type. I don't really see this as inconsistent.
I definitely think we should change equals and any other methods we find to stop using an iterator. I thought we had "no iterator" tests covering this case.
I guess we can revert the change too. These methods were returning the other types for a long time and nobody complained.
A complicated but hopefully minor issue was discovered during a workshop for Grace Hopper Celebration. A new contributor upgraded the Eclipse Collections Kata to version 12.0.0.M3 and a test began failing in a class called
MultiReaderCollectionsTest
. The methodsselect
,selectWith
,reject
,rejectWith
were were pulled up in the following PR.#1232
These methods are now returning a
MultiReaderFastList
instead of aFastList
. From an interface perspective, this is fine. Unfortunately, there is a behavior change because MultiReaderList implementations do not allow iterator to be used, so methods like equals will not work without taking an explicit lock.The simplest solution would be to return the methods to their previous state. An alternative solution would be to make sure there are covariant overrides for
select
,selectWith
,reject
,rejectWith
that returnMultiReaderList
.This is the failing test code in the Eclipse Collections Kata using 12.0.0.M3. The test was disabled as a short term resolution
https://github.com/eclipse/eclipse-collections-kata/blob/a92207659db03a72eb3f9727132637a722603073/lost-and-found-kata-solutions/src/test/java/org/eclipse/collections/lostandfoundkata/multireader/MultiReaderCollectionsTest.java#L158
This is the modified version of the code I had to write in order to get the test to pass. Note the explicit casts that were required due to the lack of covariant overrides.
I think these four methods should be reverted back in the short-term, because they are now inconsistent with all of the
collect
methods onMultiReaderFastList
which return the same type as a delegate, which would beFastList
.Thoughts?
The text was updated successfully, but these errors were encountered: