For loops vs standard library algorithms with a relatively old compiler

Question

I know code is better when there are not any confusing for loops in it. And it is always good to reuse the standard library algorithms when possible. However, I find that the syntax of iterators and algorithms looks really confusing.

I want to give a real life example from my current project: I want to copy the contents of vector<vector<QString>> in into vector<QVariant> out. I can't see the difference between:

for (int i = 0; i < in[0].size(); i++ ) 
{ 
    if(in[0][i].isNull() || in[0][i].isEmpty() ) 
        out[i] = "NONE";
    else
        out[i] = in[0][i];
}

and that:

std::transform(in[0].begin(), in[0].end(), out.begin(), [](const QString& a)->QVariant{
    if(a.isNull() || a.isEmpty() ) 
        return "NONE";
    else
        return a;
}); 

Since we have visual studio 2012 I even have to type the return value of my lambda. After using ranges like:

in[0].map!( a => a.isNull() || a.isEmpty() ? "NONE" : a ).copy(out);

in D language I simply can't live with the std::transform code above. And I am not even sure whether it is better than a basic for loop. My question is: is code using std::transform above better than the for loop?

Answer 1

At least in my opinion, the main problem here is that transform is simply the wrong tool for the job.

What you're trying to do is exactly what std::replace_copy_if does, so (no big surprise) it does it a lot more neatly.

I don't have Qt installed on the machine at hand, so I took the liberty of replacing your QVariant and QString code to just a std::vector<std::string>, but I believe the same basic idea should apply with the Qt types as well.

#include <vector>
#include <algorithm>
#include <iterator>
#include <iostream>
#include <string>

int main() {   
    std::vector<std::string> input { "one", "two", "", "three" };
    std::vector<std::string> output;

    // copy input to output, replacing the appropriate strings:
    std::replace_copy_if(input.begin(), input.end(),
                         std::back_inserter(output),
                         [](std::string const &s) { return s.empty(); }, 
                         "NONE");

    // and display output to show the results:
    std::copy(output.begin(), output.end(),
              std::ostream_iterator<std::string>(std::cout, "\n"));
}

For the moment, this just replaces empty strings with NONE, but adding the null check should be pretty trivial (with a type for which isNull is meaningful, of course).

With the data above, I get the result you'd probably expect:

one
two
NONE
three

I should probably add, however, that even this is clearly pretty verbose. It will be nice when we at least have ranges added to the standard library, so (for example) the input.begin(), input.end() can be replaced with just input. The result still probably won't be as terse as the D code you gave, but at least it reduces the verbosity somewhat (and the same applies to most other algorithms as well).

If you care about that, there are a couple of range libraries you might want to look at--Boost Range for one, and (much more interesting, in my opinion) Eric Neibler's range library.