patterncppMinor
Longest string in a vector — two implementations
Viewed 0 times
implementationstwolongeststringvector
Problem
I was given this piece of code, which calculates the length of the longest
I am new to C++ with a significant background in Java. If I would write such a function myself, I would come up with something like:
The difference is in the use of pointers, and the way in iterating the
Is there a significant performance difference between these implementations, or are there best practices involved in this?
string in a vector of strings:static size_t max_line_length(std::vector &lines) {
size_t max = 0;
for (auto it = lines.begin(); it != lines.end(); it++) {
if (it->length() > max) {
max = it->length();
}
}
return max;
}I am new to C++ with a significant background in Java. If I would write such a function myself, I would come up with something like:
static size_t max_line_length(std::vector lines) {
size_t max = 0;
for (auto line : lines) {
if (line.length() > max) {
max = line.length();
}
}
return max;
}The difference is in the use of pointers, and the way in iterating the
strings. Is there a significant performance difference between these implementations, or are there best practices involved in this?
Solution
The first version of the algorithm uses a traditional C++03-era
-
Its terse syntax (when iterators are fully spelled out, a traditional
-
Its genericity: a range-based
-
That's it for the
-
You should pass
-
In your range-based
for loop with iterators. The only modern element is the auto used to deduce an std::vector::iterator. The second loop uses a C++11 range-based for loop whose syntax is close to the one used in Java. The range-based for loop is now preferred for several reasons:-
Its terse syntax (when iterators are fully spelled out, a traditional
for loop is ugly).-
Its genericity: a range-based
for loop also works with C arrays and std::valarray.-
end is automatically cached instead of being evaluated at each iteration.That's it for the
for loops: the range-based for loop is preferred. However, there are some things that could be improved in your second version:-
You should pass
lines as a const reference. When you don't need to modify an instance, a const reference is good since it avoids a useless copy.-
In your range-based
for loop, you should write const auto& since you don't modify line either. It currently performs many useless copies of full std::strings just to get their size. Actually, the best thing to write in range-based for loops is generally auto&&, but the explanation may be a little complicated since it involves lvalues, rvalues, const-correctness and type deduction.Context
StackExchange Code Review Q#71877, answer score: 9
Revisions (0)
No revisions yet.