1

Good morning,

I am trying for a loop when a particular condition is met in an unordered_multiset with the end operation. But it does not work and I get a Segmentation fault in the next iteration of the loop.

Code:

std::unordered_multiset<Element, ElementHash>::iterator itr;
Element elementTemp1("");
for (itr = almacen.begin(); itr != almacen.end(); ++itr) {
    Element tmp(*itr);
    if(tmp.getNombre() == "prueba"){
       itr = almacen.end();
    }
}

How can I solve it? Or maybe my structure is not the right one.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
juan
  • 17
  • 5
  • sorry was a mistake when writing the question. in code is well written. – juan Dec 27 '20 at 09:25
  • Note that after the loop body is executed `++itr` is executed before testing the condition. At that time you're not at the "past end iterator position", but one past that point. – fabian Dec 27 '20 at 09:43

2 Answers2

2

What about this:

unordered_multiset<Element, ElementHash> :: iterator itr;
Element elementTemp1("");
for (itr = almacen.begin(); itr != almacen.end(); ++itr) {
Element tmp(*itr);
    if(tmp.getNombre == "prueba"){
       break;
    }
}

The break stops the execution of the for loop at that point. Is that what you want?

As mentioned by Sneftel in the comment: Your program crashes because a for-loop is executed in a way that after each iteration

  1. The increment/change operation gets executed (in your code: ++itr)
  2. The check is performed whether the loop is executed another time (in your code: itr != almacen.end()

But when you set the iterator within your loop already to almacen.end() you cannot increment it any further. But that is what happens after that iteration of the loop with Step 1 I described. And there your program crashes.

Torben Schramme
  • 2,104
  • 1
  • 16
  • 28
  • 3
    Might want to mention that the crash was happening because of incrementing the iterator after setting it to `end`. – Sneftel Dec 27 '20 at 09:24
  • thank you very much this solved the problem. But I am not convinced by the break – juan Dec 27 '20 at 12:56
  • It depends what you want. break stops the execution of the for-loop here and now. Your solution by setting the iterator (as corrected in the other answer here) is more an implicit stop by setting values in a way that the conditions to run the next iteration is are no longer met - but the current iteration is still finished. So maybe when you provided us just a stipped-down example, you actually want to execute some more code after deciding not to iterate one more time. Then, of course, break wouldn't be the correct approach. Otherwise it is. So it depends on more details of your problem :) – Torben Schramme Dec 27 '20 at 13:05
2

Your issue is that you increment end() afterward, the correct way would have been:

std::unordered_multiset<Element, ElementHash>::iterator itr;
Element elementTemp1("");
for (itr = almacen.begin(); itr != almacen.end(); /* Empty */) {
    Element tmp(*itr);
    if (tmp.getNombre() == "prueba"){
        itr = almacen.end();
    } else {
        ++itr;
    }
}

but break would be simpler, and you can also use for-range:

std::unordered_multiset<Element, ElementHash>::iterator itr;
Element elementTemp1("");
for (/*const*/auto& element : almacen) {
    if (element.getNombre() == "prueba"){
       break;
    }
}

but as-is, the loop is mostly useless.

So maybe you want to find the position where your predicate is true, then you might use std::find_if

auto itr = std::find_if(almacen.begin(), almacen.end(),
                        [](/*const*/auto& element){ return element.getNombre() == "prueba"; });
if (itr != almacen.end()) {
    /*const*/ auto& element = *itr;
    // ...
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302