Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update #282 eclipse collection version #325

Closed
wants to merge 0 commits into from

Conversation

jinjin1919
Copy link
Contributor

ready for review.

@prathasirisha
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

donraab commented Sep 22, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants