On Friday, April 08, 2016 02:01:07 Puming via Digitalmars-d-learn wrote:
> So what you mean is to read the front in constructor, and read
> further parts in the popFront()? that way multiple access to the
> front won't hurt anything. I think it might work, I'll change my
> code.
>
> So the guideline is: when accessing front is costly, don't use
> map, use a customized range struct instead. right?
In general, when you're dealing with a non-random access range, it's best
for popFront to do the work of setting up front and then have front return
the same object every time. If front is doing the work, then if it gets
called multiple times, that work is being repeated every time it gets
called. map is a funny case, because it can be a random-access range (if the
underlying range it's wrapping is a random-access range). So, fundamentally,
it doesn't work in map to do the work in popFront. It pretty much has to be
done in front. So, doing stuff like range.map!(a => to!string(a))() is
problematic in that a new allocation is going to occur every time that front
is called - or when any element is accessed via opIndex. It works so long as
the element is equal every time, and calling front multiple times does not
affect the rest of the range, but it can be costly. In theory, cache should
solve that case (and it would result in a range that wasn't random access,
so opIndex wouldn't be called on it), but obviously, you're running into
problems with it.
In any case, in general, when doing something like reading from a file with
a range, it works best to do the work in popFront to avoid issues with
multiple calls to front, and the constructor needs to do that work as well
(be it by calling popFront or not), because front needs to be valid as soon
as the range has been created, and it's not empty. So, you end up with
something like
struct MyRange
{
public:
@property T front() { return _value; }
@property bool empty() { ... }
void popFront()
{
_value = readNextValueFromFile();
}
private:
this(Something s)
{
...
popFront();
}
T _value;
}
It also encapsulates things better than having a function whose only purpose
is to be used in map, though there are obviously cases where writing a
function just to use in map would make sense.
In general, I would only use map for cases where I'm converting something to
something else and not for functions that do arbitrary work. A function for
map that cannot be pure is a danger sign IMHO. Certainly, if you're going to
follow how ranges are expected to work, whatever function you give map needs
to return equal values every time front is called between calls to popFront,
and multiple calls to front cannot affect the rest of the range. And what
you did with map, doesn't follow those guidelines, though it probably would
if cache worked, and you always fed it into cache. Still, for something
like this, I'd just create my own range and be done with it. You often need
to anyway in order to manage extra state. And it tends to be more idiomatic,
though I suppose that that's somewhat subjective.
- Jonathan M Davis
