eclipse / eclipse-collections-kata

Eclipse Collections Katas
433 stars 245 forks source link

update #282 eclipse collection version #325

Closed jinjin1919 closed 1 year ago

jinjin1919 commented 1 year ago

ready for review.

prathasirisha commented 1 year ago

@jinjin1919, please amend the commit message as there is a typo. To achieve this you can run the following two commands -

git commit --amend -m "update eclipse collections version. Closes #282" git push --force-with-lease

Thank you

prathasirisha commented 1 year ago

Hi @jinjin1919 for the failing test, I have reached out to @donraab as there some changes in versions 11.1.0 and 12.0.0.M3 but in the interest of time could you please disable the test, I will create a separate issue for @donraab and me to look at a later point.

You could add "@Disabled" to the failing test - MultiReaderCollectionsTest.multiReaderListFiltering()

donraab commented 1 year ago

Thank you, @jinjin1919. Congratulations! This failing test has uncovered a real issue. The test that is failing can be made to pass with the following code changes, but because of a recent change to MultiReaderFastList in 12.0.0.M3 requires explicit casts for the select and reject calls. The following explanation is for us committers to sort out the best solution for, but you can feel good that you found a real issue for us to fix in Eclipse Collections.

@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());
}

We removed the explicit overrides for select and reject in MultiReaderFastList in 12.0.0.M3. This is the code for the methods in 11.1.0.

    @Override
    public MutableList<T> select(Predicate<? super T> predicate)
    {
        try (LockWrapper wrapper = this.lockWrapper.acquireReadLock())
        {
            return this.delegate.select(predicate);
        }
    }

The code in 12.0.0.M3 now does this, which is implemented on MutableList.

    @Override
    default MutableList<T> select(Predicate<? super T> predicate)
    {
        return this.select(predicate, this.newEmpty());
    }

The method newEmpty() will return a MultiReaderFastList which is not safe to use with an iterator, which is why the call to withReadLockAndDelegate is required wrapping the assertEquals calls with JDK Lists.