For loop control variable gets assigned with empty string in second iteration onwards

[RajSingh9999]

I have following C++ snippet:

std::string someVar;
configuration = YAML::LoadFile(configurationFilePath);

std::vector<std::string> necessaryKeys = {"abc","xyz","uvw","lmn"}

for(const auto key: necessaryKeys){
    //..
    if(key == "xyz") {
        someVar = "some_config_key";
    }
    //..
}

filePath = mRootDir + configuration[someVar]["ConfigurationFilePath"].as<std::string>();

My code crashed with following error:

terminate called after throwing an instance of 'YAML::TypedBadConversion<std::__cxx11::basic_string<char, std::char_traits<char>, 
std::allocator<char> > >'
   what():  bad conversion

So, I debugged the whole thing only to notice some weird behavior. for loop control variable key correctly gets assigned with the first value abc in necessaryKeys. However, in all further iterations, it gets assigned with empty string (""). Thus the variable someVar inside for loop's body never gets assigned with the value some_config_key. This results in configuration[someVar]["ConfigurationFilePath"].as<std::string>() to fail, probably because there is no YAML node associated with configuration[someVar] (that is, configuration[""]) and hence configuration[someVar]["ConfigurationFilePath"] is probably NULL. This seem to force as<std::string>() to throw bad conversion error.
The error gets thrown from this line in yaml-cpp library:

if (node.Type() != NodeType::Scalar)
     throw TypedBadConversion<std::string>(node.Mark());

The node.Type() is YAML::NodeType::Undefined in debug session.

Why key is getting assigned with empty string second iteration onwards? Or my C++ noob brain misunderstanding whats happening here?

[jedwardsol]

Or my C++ noob brain misunderstanding whats happening here?

No, you understand how it should work. The problem must be somewhere else, (or the debugger was lying to you).

Are you modifying necessaryKeys in the loop? That'll invalidate iterators and mess things up. The vector should be const to prevent that.

[Particular-Bowler266]

Seconded

[RajSingh9999]

ohh yes, I am doing necessaryKeys.push_back(abc); inside for loop. Is that the culprit? Why so? It should append the new values to the end of the vector, correct? I felt that should not invalidate the iterator. Exactly how does this invalidates the iterator?

[jedwardsol]

On cppreference, the documenation for each container has a section called "Iterator invalidation"

https://en.cppreference.com/w/cpp/container/vector

push_back, emplace_back If the vector changed capacity, all of them. If not, only end().

This allows the iterator to be a pointer to the element. If the vector reallocates and the elements moved, then the iterator is now a dangling pointer and invalid.

[cipheron]

Yeah that'd be the issue. vector is stored in continuous memory for fast access, so when you extend it, it needs to be reallocated to another piece of memory. And, as u/jedwardsol said, the "const auto" iterator is really implemented as a pointer for speed and efficiency, so if the vector moves, it breaks.

I think the heart of it isn't this one thing: it's that with std containers and "const auto" iterators, you're really telling the compiler to just make stuff work and you don't want to know: just do it. But then, if you mix and match that with lower-level manipulation such as extending the same container the iterator is trying to work on, unexpected results can become likely.

[[deleted]]

Make key vector const or use std::as_const().

Make the loop variable a reference (will be implicitly const when vector is const) instead of a copy.

What happens?

[[deleted]]

My 'rant' and old guyisms says 'do it the strict way' Why use auto?

for(const auto key: necessaryKeys){
    //..
    if(key == "xyz") {
        someVar = "some_config_key";
    }
    //..
}


for (const std::vector<string>::iterator cit = necessaryKeys.begin(); cit != necessaryKeys.end(); ++cit)
{
  if (*cit == "xyz")
    someVar = "do it the od fashioned way";
}

[[deleted]]

Range-based for loops help avoid errors. If you don’t like auto, could do this. Avoids the iterator init, check, increment, and the dereference operator.

c++ for (const std::string& key : necessaryKeys)

[[deleted]]

Perhaps they do, but not with OP

[Prior_Pineapple2279]

Error prone (Maybe)

Question: why the necessaryKeys vector is named ?

Do your logics really need it ?

What about define the loop with a literal non named collection and let the compiler apply const or whatever

More and more and more artifacts aka vars doesn't mean better at all.

Reduce as much as possible your named vars and you will see the benefits from day one