front evaluated multiple time with joiner depending on where extra arg given

Jonathan M Davis jmdavisProg at gmx.com
Tue Oct 22 20:31:10 PDT 2013


On Tuesday, October 22, 2013 19:54:18 Timothee Cour wrote:
> > Certainly, I'd argue that it's generally better practice to do the work
> > in popFront, because front does frequently gets called multiple times, and
> > you
> > almost always end up calling front if you call popFront.
> 
> Actually, the way phobos is designed, often times it's easier to specify
> what front does, eg with map and related functions.

By putting the logic for calculating the element in popFront, you incur 
less of a performance hit. front will frequently get called multiple times per 
element, whereas popFront will only be called once, and it's rare that 
popFront would be called without front being called.

> > It's possible that map, joiner, and or array could use some efficiency
> > improvements, but I wouldn't consider the difference in the number of
> > calls to
> > front to be a bug. At most, it might indicate that there are some
> > optimization
> > opportunities in one or more of those functions, and it might even be that
> > the
> > differing number of calls makes sense when you actually dig into the
> > source
> > code. I'd have to go digging to know for sure.
> 
> it's not just an issue of optimization opportunities, it's an issue of
> correctness and principle of least surprise; the code I shown was simplied
> from a larger bug I had where I was doing side effects in the map lambda
> that were meant to be called once per element instead of multiple times.

It's just plain wrong for front to have side effects. You _cannot_ rely on 
front being called once per element by any particular range-based function. 
You can add side effects for debugging or whatnot, but if it has any effect on 
the behavior of your program, then it's wrong. Even if front as a getter 
property is not actually const or pure, it needs to be logically const and 
logically pure. If you want to do something once per element, then you need to 
create a wrapper that does that in popFront. popFront is the _only_ function 
that's guaranteed to be only called once per element when iterating over a 
range. And even then, if you're dealing with forward ranges, then the range 
could have been saved and have popFront called on the same element in multiple 
copies of the range, so if you're doing something in state outside of the 
range itself, then you could still be having that something happen more than 
once per element.

In general, if you want to do something once per element which involves side 
effects, I would advise using foreach rather than trying to put it into a 
range. But if you insist on doing so, the side effect should go in popFront, 
not front, or the side effects will often be happening more than once per 
element.

- Jonathan M Davis


More information about the Digitalmars-d-learn mailing list