eclipse-collections icon indicating copy to clipboard operation
eclipse-collections copied to clipboard

Implement Stack.takeWhile()

Open Saurabh-Daware opened this issue 1 year ago • 6 comments

Saurabh-Daware avatar Jul 03 '24 12:07 Saurabh-Daware

Requesting review @motlin. Thanks!

Saurabh-Daware avatar Jul 04 '24 07:07 Saurabh-Daware

The implementation is probably good, but the implementations of StackIterable.takeWhile() have no test coverage.

motlin avatar Jul 04 '24 14:07 motlin

I tried writing these tests in StackIteratable.java

@Test
    public void takeWhile(){
        StackIterable<Integer> stack = this.newStackWith(1, 2, 3, 4, 5, 6, 7);
        assertEquals( this.newStackWith(), stack.takeWhile(Predicates.alwaysFalse()));
        assertEquals(this.newStackWith(1), stack.takeWhile(each -> each <= 1));
        assertEquals(this.newStackWith(1, 2), stack.takeWhile(each -> each <= 2));
        assertEquals(this.newStackWith(1, 2, 3, 4, 5, 6), stack.takeWhile(each -> each <= stack.size() - 1));
        assertEquals(this.newStackWith(1, 2, 3, 4, 5, 6, 7), stack.takeWhile(each -> each <= stack.size()));
        assertEquals(this.newStackWith(1, 2, 3, 4, 5, 6, 7), stack.takeWhile(Predicates.alwaysTrue()));
    }

but they return

[ERROR] Failures: 
[ERROR]   ImmutableArrayStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>
[ERROR]   ArrayStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>
[ERROR]   SynchronizedStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>
[ERROR]   UnmodifiableStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>

What might be the problem?

Saurabh-Daware avatar Jul 05 '24 19:07 Saurabh-Daware

[ERROR] Failures: 
[ERROR]   ImmutableArrayStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>
[ERROR]   ArrayStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>
[ERROR]   SynchronizedStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>
[ERROR]   UnmodifiableStackTest>StackIterableTestCase.takeWhile:226 expected:<[1]> but was:<[]>

@motlin When Stack.takeWhile() was implemented using LazyIterator, returned Stack was always null. Do we have an issue with LazyIterator.takeWhile()?

Saurabh-Daware avatar Jul 16 '24 21:07 Saurabh-Daware

LGTM now, probably just needs the two commits squashed into one.

motlin avatar Jul 30 '24 23:07 motlin

Done.

Saurabh-Daware avatar Aug 04 '24 19:08 Saurabh-Daware