1

I am trying to apply the range based for loop from C++11 std in some of my existing code which has a lot of nested loops like the one below:

vector<MyClass const*> myClassVec ; 
for(vector<MyClass const*>::const_iterator itr_m(myClassVec.begin()); itr_m != myClassVec.end(); ++itr_m) {
        const MyClass* star_m(*itr_m) ;
        <<<<Do something with star_m>>>>>
        for(vector<MyClass const*>::const_iterator itr_n(itr_m +1); itr_n != myClassVec.end(); ++itr_n) {
            const MyClass* star_n(*itr_n) ;
            if (star_m->GetIndex() == star_n->GetIndex()) {
                    <<Do something>>
            }
        }
}   

I want to use auto variable and range based for loop here to make my code simpler. I was easily able to apply these to the outer loop but I am stuck at how to do this for the inner loop. Are range based loops not meant for situations like the inner for loop where you need to loop through a limited range of the container ?

The code I wrote to replace the above nested loop code and which obvious throws compile errors for the inner for loop:

vector<MyClass const*> myClassVec ; 
for(const auto& itr_m : myClassVec) {
    const MyClass* star_m(itr_m) ;
    <<<<Do something with star_m>>>>>   
    for(const auto& itr_n(itr_m +1);itr_n != myClassVec.end();++itr_n) {                
        const MyClass* star_n(*itr_n) ;
        if (star_m->GetIndex() == star_n->GetIndex()) {
            <<Do something>>
        }
    }
}

Compile error is as follows: error C2679: binary '!=' : no operator found which takes a right-hand operand of type 'std::_Vector_iterator<_Myvec>' (or there is no acceptable conversion)

Could somebody pls shed some light on how to replace the inner for loop and what are typical scenarios where the range based for loops are NOT meant to be used. Thanks in Advance!

3
  • 1
    for(const auto& itr_m : myClassVec) { <-- itr_m will not be an iterator here; it'll be a MyClass* const&. Commented Feb 24, 2016 at 10:32
  • @Michael. Have I made a mistake in the way am using it or is range based for loops always meant to give a reference to the container instead of an iterator ? Commented Feb 24, 2016 at 12:22
  • It doesn't give you a reference to the container. What you get is essentially the same as if you'd written for (std::vector<MyClass*>::iterator it = myClassVec.begin(); it != myClassVec.end(); ++it) { MyClass* const& itr_m = *it; }. The point is that you're not getting an iterator when you use a range-based for loop; you get what dereferencing the iterator would give you. Commented Feb 24, 2016 at 12:33

2 Answers 2

2

Your case actually is a typical case where range-based iteration can't be used. You can use it whenever you don't care about the position of the element/iterator in the container. Conversely, if you need the index of the current element (or an iterator pointing to it), classical loops are needed.*

See your example:

for(const auto& itr_m : myClassVec)

Here, itr_m is not an iterator, but a reference to a MyClass const*, which means that you lost all information about the position of the element.

*You could of course keep track of the current element in a range-for loop, but that would defeat its purpose and make the code harder to read.

Sign up to request clarification or add additional context in comments.

2 Comments

Thank you Anderas! I was suspecting this would be the case. I am in the process of .learning about the newer C++ features that can make my existing old code base more readable and robust. Do you have suggestions for using any other concepts here that will simplify these for loops other than using the auto variable. The need to compare elements pair wise in a container seems like a very common scenario and I would like to hear of the most efficient ways to do this.
@veekay: That would depend on what "Do something" means. It's possible that some combination of the standard algorithms would fit your needs, so that would be a good place to start looking.
0

Range based for loops may not be the best idea for what your trying to do here. Range based loops give you a reference to the object instead of an iterator.

However you can simplify them using the auto syntax to clean up the code as follows:

vector<MyClass const*> myClassVec ; 
for(auto itr_m(myClassVec.cbegin()); itr_m != myClassVec.cend(); ++itr_m) {
  const MyClass* star_m(*itr_m) ;
  <<<<Do something with star_m>>>>>
  for(auto itr_n(itr_m +1); itr_n != myClassVec.cend(); ++itr_n) {
    const MyClass* star_n(*itr_n) ;
    if (star_m->GetIndex() == star_n->GetIndex()) {
      <<Do something>>
      }
    }
}   

Trick is to use cbegin/cend to make the auto get the const_iterator you want. You could also go a step further with auto to make the code easier to read, but it's just a matter of style at this point.

vector<MyClass const*> myClassVec ; 
for(auto itr_m(myClassVec.cbegin()); itr_m != myClassVec.cend(); ++itr_m) {
  auto star_m(*itr_m) ;
  <<<<Do something with star_m>>>>>
  for(auto itr_n(itr_m +1); itr_n != myClassVec.cend(); ++itr_n) {
    auto star_n(*itr_n) ;
    if (star_m->GetIndex() == star_n->GetIndex()) {
      <<Do something>>
      }
    }
}   

2 Comments

Thank you Atif for pointing out the use of cbegin and cend. I agree those are safer to use in this case to get the exact replacement of my original code.
Yeah I would think of the range-based for loop as a foreach. It's great if all you are doing is processing each element in the same way, however it gets rid of the iterator, so additional iterator arithmetic isn't possible. Style wise its a nice distinction. Simple for loops, turn into range-based for, and the rest use iterators signifying something more if being done. Please upvote the answer if you like the feedback :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.