Here is an iteration style that works as is with today's behaviour of
hasNext
1.
2. Element thisElement = null;
3. Element thatElement = null;
4. boolean advanceThis = true;
5. boolean advanceThat = true;
6.
7. Iterator<Element> thisNonZero = this.iterateNonZero();
8. Iterator<Element> thatNonZero = x.iterateNonZero();
9.
10. double result = 0.0;
11. while (true) {
12. *if (advanceThis) {
*
13. * if (!thisNonZero.hasNext()) {
*
14. * break;
*
15. * }
*
16. * thisElement = thisNonZero.next();
*
17. * }
*
18. * if (advanceThat) {
*
19. * if (!thatNonZero.hasNext()) {
*
20. * break;
*
21. * }
*
22. * thatElement = thatNonZero.next();
*
23. * }*
24. if (thisElement.index() == thatElement.index()) {
25.
26. result += thisElement.get() * thatElement.get();
27. advanceThis = true;
28. advanceThat = true;
29. } else if (thisElement.index() < thatElement.index()) {
30. advanceThis = true;
31. advanceThat = false;
32. } else {
33. advanceThis = false;
34. advanceThat = true;
35. }
36. }
On Sat, Apr 13, 2013 at 1:47 AM, Ted Dunning <[email protected]> wrote:
> The caller is not at fault here. The problem is that hasNext is advancing
> the iterator due to a side effect. The side effect is impossible to avoid
> at the level of the caller.
>
> Sent from my iPhone
>
> On Apr 12, 2013, at 12:22, Sean Owen <[email protected]> wrote:
>
> > I'm sure I did (at least much of) the AbstractIterator change so blame
> > me... but I think the pattern itself is just fine. It's used in many
> > places in the project. Reusing the value object is a big win in some
> > places. Allocating objects is fast but a trillion of them still adds
> > up.
> >
> > It does contain a requirement, and that is that the caller is supposed
> > to copy/clone the value if it will be used at all after the next
> > iterator operation. That's the 0th option, to just fix the caller
> > here.
> >
> > On Fri, Apr 12, 2013 at 7:49 PM, Ted Dunning <[email protected]>
> wrote:
> >> The contract of computeNext is that there are no side effects visible
> >> outside (i.e. apparent functional style). This is required since
> >> computeNext is called from hasNext().
> >>
> >> We are using a side-effecting style so we have a bug.
> >>
> >> We have two choices:
> >>
> >> a) use functional style. This will *require* that we allocate a new
> >> container element on every call to computeNext. This is best for the
> user
> >> because they will have fewer surprising bugs due to reuse. If
> allocation
> >> is actually as bad as some people think (I remain skeptical of that
> without
> >> tests) then this is a bad move. If allocation of totally ephemeral
> objects
> >> is as cheap as I think, then this would be a good move.
> >>
> >> b) stop using AbstractIterator and continue with the re-use style. And
> add
> >> a comment to prevent a bright spark from reverting this change. (I
> suspect
> >> that the bright spark who did this in the first place was me so I can be
> >> rude)
>
