Thursday, September 18, 2014

Common pitfall of new C++11 for-auto idiom

C++11 has brought in some developer friendly extensions. One of my most used ones is:

for(auto item : container) {
   // process item
}

this typically replaces the following clunky looking iteration pattern:

typedef std::list myIntList;
myIntList theList;
// skip ... code that populates the list

//iterate over list doing some process
for(std::list::const_iterator iter = theList.cbegin(); iter != theList.cend() ; ++ iter) {
   doSomething(iter);
   doSomethingToObject(*iter);
}


The constant repetition of the iterator definition was just busy work that didn't add to readability or efficiency. C++11 recognized and simplified the above software pattern to:


for(auto iter = theList.begin(); iter != theList.end() ; ++iter) {
   doSomething(tier);
   doSomethingToObject(*iter);
}


alternatively, in many cases the following truncation "works" too:

for(auto item : theList) {
   doSomethingToObject(item);
}


I qualify "works" because above is perfectly fine for very simple elements, but for larger elements and lists can incur a significant temporary allocation penalty because of a subtlety: item is a copy-constructed temp of a corresponding item in theList. This can lead to many more allocation,constructor and destructor,deallocation. Instead C++11 also provides:

for(auto & item : theList) {
   doSomething(item);
}


and

for(const auto & item : theList) {
   doSomething(item);
}


which still access and process each element in the container but use a reference, const reference, respectively. This has the advantage of avoiding the construct-alloc-destruct-dealloc penalty.

There is also a greater disadvantage in the first instance, in which doSomething accepts a non-const lvalue reference: the operation is on a temp and is non-persistent past the scope of the doSomething(item) statement.

Not seeing this can lead to several minutes/hours/days of consternation as your code would seem to have no effect. Alternatively, your bugs might be missed if all appears falsely-well.

It is now my personal preference to expect, enforce and use the idiom:

for(const auto & item : theList) {
   doSomething(item);
}


unless there's a compelling reason to use another approach instead. And even then to question before accepting.

references:
http://www.cplusplus.com/reference/list/list/cbegin/

No comments: