avoiding copying of iterator to const_iterator

[subramanian100in]

Consider the code fragment:

vector<int> container;
container.push_back(0);
container.push_back(1);
container.push_back(2);

Now I want to iterate through the 'container'.

For this, instead of writing,

for (vector<int>::const_iterator cit = container.begin();
cit != container.end();
++cit)
// ... do something ...

I thought I should write:

const vector<int>& ref = container;

for (vector<int>::const_iterator cit = ref.begin();
cit != ref.end();
++cit)
// ... do something ...

The reason is :
in the line vector<int>::const_iterator cit = container.begin();
the 'container' is non-const. So 'container.begin()' will return
plain iterator which is copied to const_iterator. So a
conversion is involved here.

Instead, if I use ref.begin() - with 'ref' as defined above,
'begin() const' will be called on 'ref' which will directly return
a const_iterator. So no conversion is involved here.

Is my understanding correct ?

Also, kindly clarify if the line
const vector<int>& ref = container;
involves any temporary object ?

Thanks
V.Subramanian

 

[Ian Collins]

Consider the code fragment:

vector<int> container;
container.push_back(0);
container.push_back(1);
container.push_back(2);

Now I want to iterate through the 'container'.

For this, instead of writing,

for (vector<int>::const_iterator cit = container.begin();
cit != container.end();
++cit)
// ... do something ...

I thought I should write:

const vector<int>& ref = container;

for (vector<int>::const_iterator cit = ref.begin();
cit != ref.end();
++cit)
// ... do something ...

The reason is :
in the line vector<int>::const_iterator cit = container.begin();
the 'container' is non-const. So 'container.begin()' will return
plain iterator which is copied to const_iterator. So a
conversion is involved here.

Why bother? The original is idiomatic, the second is not.

Any conversion will be trivial and optimised. std::vector iterator may
well be a plain pointer.

Also, kindly clarify if the line
const vector<int>& ref = container;
involves any temporary object ?

No.

 

[Jerry Coffin]

[ ... ]

Now I want to iterate through the 'container'.

Usually, you want to do no such thing. Rather, you want to apply an
algorithm to the container...

For this, instead of writing,

for (vector<int>::const_iterator cit = container.begin();
cit != container.end();
++cit)
// ... do something ...

I thought I should write:

const vector<int>& ref = container;

I think this is pointless. At least conceptually, a const_iterator acts
a bit like a base class of iterator -- i.e. such a conversion is always
reasonable and safe. In any case, this doesn't really change anything:
you're just substituting one conversion for another.

for (vector<int>::const_iterator cit = ref.begin();
cit != ref.end();
++cit)
// ... do something ...



The reason is :
in the line vector<int>::const_iterator cit = container.begin();
the 'container' is non-const. So 'container.begin()' will return
plain iterator which is copied to const_iterator. So a
conversion is involved here.

Instead, if I use ref.begin() - with 'ref' as defined above,
'begin() const' will be called on 'ref' which will directly return
a const_iterator. So no conversion is involved here.

Yes and no -- the line that obtains the iterator no longer uses a
conversion, but the line that forms the reference _does_ -- and it's
basically the same conversion. IOW, you've taken simple, easily
recognizable code and converted it to something more complex and less
idiomatic, but retained the same basic problem.

If you really want to improve the code, look at the 'do something' part,
and figure out which algorithm to use instead of your hand-coded loop.
Chances are you'll quickly realize that you can substituted
std::for_each almost verbatim:

std::for_each(container.begin(), container.end(), do_something);

but you should fight this urge. More often than not, another algorithm
will work better and the result will be cleaner and more readable.

 

[Pete Becker]

Instead, if I use ref.begin() - with 'ref' as defined above,
'begin() const' will be called on 'ref' which will directly return
a const_iterator. So no conversion is involved here.

Yes and no -- the line that obtains the iterator no longer uses a
conversion, but the line that forms the reference _does_ -- and it's
basically the same conversion. IOW, you've taken simple, easily
recognizable code and converted it to something more complex and less
idiomatic, but retained the same basic problem.
[/QUOTE]

Containers in C++0x will have member functions cbegin(), cend(),
crbegin(), and crend() for exactly this problem. They return
const_iterators, even from non-const containers.

 

[James Kanze]

[ ... ]

Now I want to iterate through the 'container'.

Usually, you want to do no such thing. Rather, you want to apply an
algorithm to the container...

Sort of. The algorithm may be unique and dependent on the
context.

I think this is pointless. At least conceptually, a const_iterator acts
a bit like a base class of iterator -- i.e. such a conversion is always
reasonable and safe. In any case, this doesn't really change anything:
you're just substituting one conversion for another.

Yes and no -- the line that obtains the iterator no longer uses a
conversion, but the line that forms the reference _does_ -- and it's
basically the same conversion.

Not really. Typically, it will be handled completely at compile
time---the compiler will simply note that ref is an alias for
the vector, and call the const versions of the functions rather
than the non-const ones when the call goes through ref.

It's still obfuscation, though, and not worth the bother.

IOW, you've taken simple, easily recognizable code and
converted it to something more complex and less idiomatic, but
retained the same basic problem.

Supposing that there was a problem to begin with. Personally, I
don't see one.

If you really want to improve the code, look at the 'do
something' part, and figure out which algorithm to use instead
of your hand-coded loop. Chances are you'll quickly realize
that you can substituted std::for_each almost verbatim:

std::for_each(container.begin(), container.end(), do_something);

It doesn't work out in practice that often. Usually, you'll
have to write a custom do_something object, and often, you'll
have to pass it additional state. Such a procedure is valid if
do_something lends itself to a good name, of course.

but you should fight this urge. More often than not, another
algorithm will work better and the result will be cleaner and
more readable.

I've not found that to be the case that often. I do watch out
for cases where I can reasonably give a name to the operation,
and especially where I'm likely to use variants of it in many
different places, and write functional objects in those cases,
but there are a still a lot of cases where it doesn't apply.

I might add that while the original poster seems worried about
the single conversion at the top of the loop, he has a
conversion in the loop every time he compares with the results
of end(). In some cases, this can affect performance---not so
much the conversion, of course, but the call to end()---, and
caching the results of end() in a separate variable may be worth
considering if the profiler says you have a performance problem
here.