RangeOf iterator

[neonailol]

It would be nice to have RangeOf implementation as Iterator Following code probably can be refactored into this new implementation

https://github.com/yegor256/cactoos/blob/c6ca6d183ee5d0ca7dd76701231333af611a6439/src/main/java/org/cactoos/iterable/RangeOf.java#L50-L73

[0crat]

@llorllale/z please, pay attention to this issue

[0crat]

@neonailol/z this project will fix the problem faster if you donate a few dollars to it; just click here and pay via Stripe, it's very fast, convenient and appreciated; thanks a lot!

[svendiedrichsen]

@neonailol What is wrong with calling the iterator() method of the RangeOf instance? It is like a factory for iterators.

[neonailol]

@svendiedrichsen well, it supposed to be the other way around, all iterable classes are built on top of iterators classes. Also if I recall correctly, anonymous inner classes tend to pollute memory, but I am not sure.

[svendiedrichsen]

@neonailol Iterators are stateful and kind of a one-shot thing. Using and working with iterators is an errorprone style. You may run into situations where you will have to switch back and forth between iterable and iterator. Please see #677 which should have removed the explicit usage of iterators especially as parameters to ctors. IMHO a framework especially when it is lazy in evaluation should rather use iterables to avoid those kind of situations and make the handling more predictable. Do you really think that Cactoos has a shortage of anonymous inner classes when one key concept is the usage of Scalars?

[neonailol]

@svendiedrichsen

Please see #677 which should have removed the explicit usage of iterators especially as parameters to ctors.

I don't see how that issue correlates with this one. Are you saying that all classes from org.cactoos.iterator should be removed? From what I see: all iterables are built on top of iterators, why this class should be different?

[svendiedrichsen]

Are you saying that all classes from org.cactoos.iterator should be removed?

@neonailol: IMHO at least they should not be visible.

Thesis 1: Stateful instances in functional/ lazy evaluating code is evil/ hard to get right. Thesis 2: Public classes will be used. Conclusion: Don't create public stateful classes in functional frameworks or encourage their usage.

Iterators are a nessessity for looping in Java but IMHO should not be used for anything else. So don't encourage anyone by providing public Iterator implementations or have interfaces/methods require them.

Sorry if this came across a little harsh. This is just my opinion.

[neonailol]

@svendiedrichsen You have good points, I'll just leave it to an architect to decide, whether it should or should not be done.

Do you really think that Cactoos has a shortage of anonymous inner classes when one key concept is the usage of Scalars?

Scalars effectively used as lambdas thought the code, and lambdas work differently from anonymous classes, basically, lambdas become static methods in the end. Whereas anonymous inner classes require an object instantiation, small runtime cost at first use, and their own scope, where they capture enclosing class.

[svendiedrichsen]

... lambdas become static methods in the end ...

@neonailol Ok, I didn't know that. Good point then.

[llorllale]

@svendiedrichsen we already have public Iterators. Have you found any issue with them?

[svendiedrichsen]

@llorllale Yes, conceptionally. Nothing personal ;). I think I have covered this enough in the above discussion. Besides this there were several bugs like i.e. #659.

[neonailol]

The problem not in iterators, the problem are in constructors that take iterator as a parameter, so probably all constructor like this #905 should be removed, or maybe wrap this iterator to some sticky iterable decorator, that caches values returned from the iterator (this does not sound like a good idea tho)

[svendiedrichsen]

@neonailol You are almost there. Iterators aren't conceptually bad but they don't fit in a stateless framework. You can work around each problematic situation specifically or get rid of their explicit usage at all. I think the latter. Thats why I think they shouldn't be used anywhere in Cactoos interfaces/methods/ctors. You can use them in implementation code tho but create them from iterables on demand, use them once and forget them.

[llorllale]

@svendiedrichsen

Nothing personal

No offense was taken. You raise good points.

I am torn between the issues you raise and that ugly code that @neonailol pointed out. It would be much better if you could provide that iterator in a declarative manner.

Plus there's the fact that we have tons of implementations of Iterator as part of our public API. That's why I asked if there was some specific issue you found among any of them.

[svendiedrichsen]

@llorllale There is no technical issue with the iterators in the API. There is only IMHO a conceptual problem which for example manifests in bugs like #659 or #905. This problem makes the usage of the API unpredictable if used extensively.

A solution may be to replace the Iterator usages in the public API with the usage of Iterable and have all Iterators package private. The API is still not 1.0 version. So it may still change incompatibly.

You better get some more opinions on this. Maybe @yegor256 ?

[llorllale]

@svendiedrichsen replacing use of Iterator with Iterable won't solve the problem when you can still do things like

new IterableOf<>(new IteratorOf<>(...));

This problem can perhaps be solved by:

• Process: we failed to add tests to the relevant ctors of both Cycled
• Technical: both Cycled should only accept scalars and be clearly documented that these scalars need to supply fresh instances of the iterator.

WDYT?

[svendiedrichsen]

@llorllale

replacing use of Iterator with Iterable won't solve the problem when you can still do things like

If you followed my suggestions you can't do new IterableOf<>(new IteratorOf<>()). The ctor accepting an iterator won't exist anymore and the IteratorOf won't be publicly available.

Just to repeat myself: Just remove any explicit usages of all Iterator as parameter and hide any Iterator implemenation and you will be better off.

[llorllale]

@svendiedrichsen

have all Iterators package private

That is something I definitely don't want to do. Those iterators are useful.

I'm trying to find a middle ground here. My suggestion is to leave ctors that accept Scalar<Iterator<T>> and document that new instances of Iterator should be created on every call to Scalar.value(). If users don't pay attention to this doc then this would suffer from the same problem:

final Iterator<?> iter = ...;
new ListOf<>(() -> iter);  //not providing new instance on every call to Scalar.value()

Incidentally, your suggestion to replace all use of Iterator with Iterable will suffer from the same issues once you realize that Iterable can be implemented as the lambda () -> iterator. Unless you do () -> new IteratorOf<>(...).

[svendiedrichsen]

@llorllale

Those iterators are useful.

True. But they must be wrapped by an Iterable to assure that each call to iterator creates a new instance of Iterator.

Where is the difference between Scalar<Iterator<T>> and Iterable<T>? IMHO the suggestion is valid to avoid the usage of Iterator. Although the javadoc of Iterable does not state directly to create new Iterator instances upon each call to the iterator() method but each implementation does it.

Here is the question discussed on Stackoverflow. https://stackoverflow.com/questions/15781739/is-there-any-official-contract-for-the-iterable-interface-with-respect-to-multip

I would not recommend to rely on developers reading javadoc to avoid errors when using the API. Simply force it by using Iterable.

The API is still in pre 1.0 version. So incompatible changes may happen.

[llorllale]

@svendiedrichsen

Where is the difference between Scalar<Iterator> and Iterable?

There is none. I hadn't realized they're the same thing in this context - even when I wrote my previous comment.

So here are my decisions:

• We will keep org.cactoos.iterator as part of our public API
• I am accepting this new requirement from @neonailol
• We will create a new ticket to continue where #677 left off. Can you please create it, @svendiedrichsen ?

[llorllale]

@0crat in

[0crat]

There is an unrecoverable failure on my side. Please, submit it here:

PID: 4@6b5bb75a-1f3d-4df6-874b-c80cf409a2f7, thread: pool-23-thread-1
com.zerocracy.farm.sync.SyncProject[77] java.lang.IllegalStateException: Failed to acquire "people.xml" in "PMO" in 1min: 99f8085adfb7/46s/0/5/true/false by QuartzScheduler_Worker-9

0.22.10: Issue: yegor256/cactoos#906, Comment: 398750128

[llorllale]

@0crat in

[0crat]

There is an unrecoverable failure on my side. Please, submit it here:

PID: 4@a3ae9e3a-5c55-4597-a751-c76188d0d54a, thread: pool-53-thread-1
com.zerocracy.farm.sync.SyncProject[77] java.lang.IllegalStateException: Failed to acquire "people.xml" in "PMO" in 1min: 5a7468575572/8min/0/2/true/false by QuartzScheduler_Worker-10

0.22.11: Issue: yegor256/cactoos#906, Comment: 398770441

[svendiedrichsen]

@llorllale Wouldn't it be better to create tickets for each replacement? Thus you don't have such big PRs to review and you can split the job among several devs.

[llorllale]

@svendiedrichsen PDD is expected to be used

[svendiedrichsen]

@llorllale Do you expect a ticket that requests to create puzzles throughout the codebase?

[llorllale]

@svendiedrichsen I expect a ticket that will ask to "substitute all use of Iterator with Iterable in all ctors" and point to this discussion for context.

[llorllale]

@0crat in

[0crat]

There is an unrecoverable failure on my side. Please, submit it here:

PID: 4@aa0fe21a-ccc8-4a0f-920f-f4c2aaf4a20f, thread: pool-177-thread-1
com.zerocracy.farm.sync.SyncProject[77] java.lang.IllegalStateException: Failed to acquire "people.xml" in "PMO" in 2min: 6cc77ffce735/31min/0/0/false/false by zerocrat_bot Telegram Executor

0.22.25: Issue: yegor256/cactoos#906, Comment: 399091581

[llorllale]

@0crat in

[0crat]

@0crat in (here)

@llorllale Job #906 is now in scope, role is DEV

[0crat]

Bug was reported, see §29: +15 point(s) just awarded to @neonailol/z

[0crat]

The job #906 assigned to @longstone/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this; there will be no monetary reward for this job

[0crat]

The architect of the project has changed; @llorllale/z is not at this role anymore; @paulodamaso/z is the architect now

[0crat]

The architect of the project has changed; @paulodamaso/z is not at this role anymore; @victornoel/z is the architect now

[victornoel]

@0crat in

[0crat]

The job #906 assigned to @borysfan/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this; there will be no monetary reward for this job

[victornoel]

@0crat refuse

[0crat]

@0crat refuse (here)

@victornoel The user @borysfan/z resigned from #906, please stop working. Reason for job resignation: Order was cancelled

[victornoel]

@neonailol done in #1542

[0crat]

@sereshqua/z please review this job completed by @baudoliver7/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

[sereshqua]

@0crat quality acceptable

[sereshqua]

see