Inconsistent interfaces between 'Paged' classes

yegor256 / cactoos

Object-Oriented Java primitives, as an alternative to Google Guava and Apache Commons

https://www.cactoos.org

MIT License

737 stars 163 forks source link

Inconsistent interfaces between 'Paged' classes #1595

Closed SergiusAC closed 3 years ago

SergiusAC commented 3 years ago

Issue #1538 says next: https://github.com/yegor256/cactoos/blob/86ad37b671edb86a5d92d00c1005cc6c62f2c825/src/main/java/org/cactoos/iterable/Paged.java#L38-L42 But this replacing is inconsistent with the interface in the next class: https://github.com/yegor256/cactoos/blob/86ad37b671edb86a5d92d00c1005cc6c62f2c825/src/main/java/org/cactoos/iterator/Paged.java#L42-L67 This inconsistency prevents issues #1538 and #1537 from being completed.

victornoel commented 3 years ago

@SergiusAC I don't understand what exactly is the problem. The two todos seem to be two orthogonal concerns. The first one is about parameter order in the constructor and the second one is about removing a useless type parameter (generic) of the class.

SergiusAC commented 3 years ago

@victornoel the iterable.Paged constructor should take the parameter 'next' as 'Func<Iterable, Iterable>', it is mentioned in first todo. But the iterator.Paged constructor takes the corresponding parameter as 'Func<Iterator, Iterator>'. So we cannot invoke the iterator.Paged constructor and pass it the parameter 'next'.

victornoel commented 3 years ago

@SergiusAC well, this is the purpose of the task to find a solution. I see you refused the task, so maybe let it for the next person it will be assigned to. I don't see any problem with it, most of the Iterable of Cactoos are implemented by delegating to the corresponding Iterator implementation, so it should be doable. I don't have the solution exactly, but if I had, I would have done the job.

If I understand well, it seems your question is about #1537, not about #1538 (which is just about switching two parameters in a constructor…), so I think next time, you can directly discuss it in the ticket of the problematic case :)

victornoel commented 3 years ago

@SergiusAC oh wait, I'm mistaken, the question is indeed about #1538, sorry. Anyway, my answer stands, we should find a way to take an iterable instead of an iterator in iterable.Paged.

victornoel commented 3 years ago

@SergiusAC re-reading my answers, maybe something is not clear:

the objective of #1538 is to have the constructor of iterable.Paged take a Func<Iterable, Iterable> but invoke iterator.Paged with a Func<Iterator, Iterator>, like it is done in all the other Iterable implementations of Cactoos. The solution is not obvious but there is not reason not to be possible, most certainly using func.Mapped or something like that.
the objective of #1537 is to simplify the implementation of the class of iterator.Paged but still passing a Func<Iterator, Iterator> to the constructor

I believe your question was thus about #1538. I propose that if all is clear to you, we close this ticket and I will add a link to this discussion in #1538 for the next person to tackle it :) or if you want to tackle it I can re-assign it to you with no problem :)

SergiusAC commented 3 years ago

@victornoel I think this ticket can be closed, because solution to make interfaces between the classes consistent can be done like this

new org.cactoos.iterator.Paged<X>(
    first.iterator(),
    page -> next.apply(new IterableOf<>(page)).iterator()
)

Also, if it is possible, then re-assign #1538 to me again, please.

victornoel commented 3 years ago

@SergiusAC exactly, good solution

0crat commented 3 years ago

Job gh:yegor256/cactoos#1595 is not assigned, can't get performer