Skip to content

Collections-699 PairingIterator#74

Open
Fival85 wants to merge 6 commits into
apache:masterfrom
Fival85:COLLECTIONS-699
Open

Collections-699 PairingIterator#74
Fival85 wants to merge 6 commits into
apache:masterfrom
Fival85:COLLECTIONS-699

Conversation

@Fival85

@Fival85 Fival85 commented May 22, 2019

Copy link
Copy Markdown

Hello again,

like in #73 here is the second try to implement COLLECTIONS-699

Now i use a standalone lightweight dto inside the PairingIterator to return the result.

Is the implementation ok?

Fival85 added 5 commits May 21, 2019 19:56
[NEW] An iterator for returning pairs of the childs iterators, askes in COLLECTIONS-699
[FIXED] replacing tabs with whitespaces
Instead of a common-lang Pair we use a custom lightweight dto.
Change dependency scope of commons-lang3 back to "test"
@coveralls

coveralls commented May 22, 2019

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 87.881% when pulling ba54ebc on Fival85:COLLECTIONS-699 into 252170f on apache:master.

unit tests for equals and toString of the dto entry

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Fival85,

Thank you for your patch. Please address the comments below/above.

Gary

if (!hasNext()) {
throw new NoSuchElementException();
}
final F firstValue = null != firstIterator && firstIterator.hasNext() ? firstIterator.next() : null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not seem necessary to call each delegate iterator's hasNext() method since this method just invoked its own hasNext().

In general, this begs the question of what thread-safety guarantees this class makes. If none, the Javadocs should say so.

* If one iterator has more elements then the other, the result {@link Entry}
* will contain a null value and the value of the not empty child iterator until
* both of the iterator exhausted.
*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing a close tag for 'p' here.

* both of the iterator exhausted.
*
* @param <F> type of first value. The first value of {@link Entry}
*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blank line is not needed.

* @param <F> the type of the first element
* @param <S> the type of the second element
*/
public static class Entry<F, S> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed if you reuse one of:

  • org.apache.commons.collections4.keyvalue.DefaultKeyValue<K, V>
  • org.apache.commons.collections4.keyvalue.DefaultMapEntry<K, V>
  • org.apache.commons.collections4.keyvalue.MultiKey<K>


/**
* Unit test suite for {@link PairingIterator}.
*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra line not needed ;-)

@Fival85

Fival85 commented Jul 26, 2019

Copy link
Copy Markdown
Author

Thank you. I will check it, when i have time for it. at the moment im on vacation :)

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