denis bider

Various thoughts

"auto" considered (often) harmful

Posted Nov 22, 2015 at 19:42 by denisbider

Last edited Nov 23, 2015 at 15:29 by denisbider

cpp programming

Edit: Thanks to Drammon, Simon Brand, and Nicola Gigante for important corrections (see comments).

It's now in vogue to write C++ code like this:

 

 auto const& container = Function();
 for (auto const& element : container)
 {
 auto const& member = element.AccessMember();
 ...
 }

const& is necessary because auto strips references and const/volatile qualifiers. This is good: it is apparent when you're not copying a whole object, even if its type is hidden from view.

But please don't do this (too much).

The value of strong typing in C++ is not only in ensuring consistency at compile-time, it's also to document what the code is doing.

The above snippet requires that the reader knows:

what Function returns
what container.begin() or begin(container) returns
what element.AccessMember() returns

in order to just know that the code is doing this:

 
 Container const& container = Function();
 for (Container::Element const& element : container)
 {
 Member const& member = element.AccessMember();
 ...
 }

This reduces readability of the code. By using auto this way, you're throwing away an important self-documenting property of the language.

There's an argument that this improves maintainability because if the return value of Function changes to a type that behaves the same, you need to make fewer code changes.

But maintainability is not just about reducing code changes; it's about ensuring their correctness. A developer needs to understand the changes they are making, and that a change is propagated correctly throughout the program. auto makes code harder to understand, and hides places affected by changes.

auto is definitely needed with lambdas:

 
 auto lambda = [&] (Seq x) -> bool
 {
 ...
 };

In this case, auto is the right thing to do – otherwise, you're wrapping the unnamed, compiler-specific raw lambda type into an std::function, and doubly declaring the function signature.

If you find that the classes you are using require really obtuse syntax to use explicit types:

 
 std::map<unsigned int, std::string> const& container = Function();
 for (std::map<unsigned int, std::string>::value_type const& element : container)
 ...

... then maybe that's the fault of an unfriendly design of the library you are using. In cases like that, I much prefer that, instead of auto, we use a suitable type alias:

 
 using MyMap = std::map<unsigned int, std::string>;

 MyMap const& container = Function();
 for (MyMap::value_type const& element : container)
 ...

List older posts

Showing 11 out of 11 comments, oldest first:

Comment on Nov 23, 2015 at 10:52 by Drammon

using auto drops references, const qualifiers, and volatile qualifiers
see http://herbsutter.com/2013/06/07/gotw-92-solution-auto-variables-part-1/

Comment on Nov 23, 2015 at 10:59 by Unknown

Yeah, for auto container = Function(); container won't be a reference, you'd need to write const auto& container = Function(); for that. decltype(auto) acts in the way you've outlined.

On the maintainability side, what if the type Function returns changes? If you use auto, you don't need to do anything (so long as the interface is compatible). If you use explicit types, you need to update everywhere. Sure, maybe you designed for change and made it an alias/typedef, but that's still another thing to remember/forget.

Comment on Nov 23, 2015 at 11:04 by Unknown

"In this case, auto really is reasonable, otherwise you're duplicating a type signature you just wrote." What do you mean by that? You can't really duplicate the type signature of a closure as it's implementation-defined.

Comment on Nov 23, 2015 at 11:08 by Unknown

As someone already pointed out, at the beginning of the post you are overlooking that the auto keyword always discards references and cv-qualifiers, so the second interpretation of the code is not possible.

Also, beware that the use of the auto keyword in the example with the lambda expression is "required", not just "useful" to avoid repetition of the signature. The type of the lambda expression is unnamed, so that you cannot in any way declare a variable of the right type without auto. In case you were thinking about std::function<>, that is not the type of the lambda. It is a polymorphic wrapper around callable objects, which adds runtime overhead to do type erasure.

Comment on Nov 23, 2015 at 13:18 by denisbider

Looks like you guys caught me with my pants down! Thank you for your comments. I should do my homework before posting. :)

I have updated the post as follows:

- Corrected examples to take auto's ref/cv stripping into account.

- Clarified the impact I think auto has on readability and maintainability.

- Clarified the loose language around "duplicating" a lambda's type signature, pointing out this also wraps the raw unnamed lambda type into an std::function.

Thanks for pointing out these inaccuracies. Especially the const& is a big one. :)

Comment on Nov 23, 2015 at 13:59 by Unknown

This is mostly a difference is philosophy, but I think forgetting about the concrete types of the variables and focusing on the behaviour and implicit interfaces is better than locking the source to a given type set.

auto const& container = Function();
for (auto const& element : container)
{
auto const& member = element.AccessMember();
...
}

Yes, that code is unclear, but I think better naming solves that issue without resorting to full type annotations:

auto const& students = GetStudents();
for (auto const& student : students)
{
auto const& grade = student.GetGrade();
}

Now the code is pretty clear. GetStudents returns some collection of students. Is it a std::vector? A std::set? A custom::MySpecialCollection? Who cares? So long as we can iterate over it to access the students, we don't need to think about it. If we then change the container to a smart::SuperOptimizedCollection then we maintain our clarity and don't need to touch that part of the source. If we profile our code and find that this part is slow, we can look closer at the type and work out if we're doing something inefficient.

We forget about concrete types all the time. Every time you use an iterator for example. What is std::vector::iterator? Who cares? So long as it allows us to iterate over ints without doing anything ridiculous then we're happy.

Comment on Nov 23, 2015 at 14:12 by denisbider

Ideally, we can have the advantages of both worlds - clarity as well as auto-adjustment - with tools that show the underlying types. VS 2015 does this - you can hover over a variable and see what it is - but it only works if you have the full solution loaded, not when you want to quickly look up some file. And although it has become quite reliable, it doesn't work always.

Note also, if you change the underlying organization, you will still need to make changes to the code. It seems to me much more likely that you would change from std::vector to std::map, rather than from std::vector to std::same_interface_as_vector_but_faster. If this second class with the same interface as vector were miraculously faster, it would probably be vector. :)

But if you change the underlying organization, then code relying on auto needs to change, too.

Comment on Nov 23, 2015 at 18:53 by Musteresel

I'd agree with you that using typedefs with descriptive names is often the better choice than using auto when facing long an complex types. But regarding your critique points:

"The above snippet requires that the reader knows [..] "

This is only true when you have such generic function names as "Function" or "memberAccess". With descriptive names for functions and auto one can hide implementation details such as whether the collection is a vector or a list but still write self documenting code.

Comment on Nov 23, 2015 at 19:06 by denisbider

I used "Function" because to me it doesn't matter whether it's "Students" or "Countries". The information I want to see is whether it's a vector, or a set, or what. We're implementing, so I want the implementation details.

It's as if you opened the hood of a car; but under the hood, there's another hood, to hide the implementation details. :)

Comment on Nov 23, 2015 at 19:11 by denisbider

One might argue that by using a type alias I'm also hiding implementation details. But I would not propose defining an alias like this:

using Students = std::map<unsigned int, Student>;

Instead, I suggest this:

using StudentsById = std::map<unsigned int, Student>;

Or this:

using StudentsByName = std::map<std::string, Student>;

Comment on Jan 22, 2016 at 04:13 by Unknown

Don't know why people would write code "in vogue". Honestly as long as my codes help me get my finances in order and don't return a NULL value of sorts, I'm happy as a lark!