mahout-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dan Filimon <dangeorge.fili...@gmail.com>
Subject Re: Odd vector iteration behavior
Date Fri, 12 Apr 2013 19:31:32 GMT
The problem with the 0th option is that the elements provide no way of
cloning them.

Also, reusing the value causes the problem in the first post, where the
element advances even though it shouldn't.
Calling next(), keeping the reference and expecting the value to not change
when calling hasNext() seems reasonable to me.

It seems non-intuitive that it should be cloned, because calling hasNext()
could invalidate it. In my mind, the current element should only change
when calling next().



On Fri, Apr 12, 2013 at 10:22 PM, Sean Owen <srowen@gmail.com> 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 <ted.dunning@gmail.com>
> 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)
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message