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)
