12 ms·
auto y = std::move(x); Is this good code? How do you review it? What's the type of y? Usually I would ask for at least one type statement per line. For exa
by thrownaway2424 12y ago
auto y = std::move(x);
Is this good code? How do you review it? What's the type of y? Usually I would ask for at least one type statement per line. For example this seems ok to me, because the type is on the left.
auto* x = new Foo();
But this doesn't seem ok:
auto y = std::move(x);
- fleitz 12y agoThe types are immaterial to the operation, the important thing is the move, not the type line noise.
- octo_t 12y agowhat about in the case of something like: auto it = std::find(vec.begin(), vec.end(), value);
- thrownaway2424 12y agoThat doesn't bother me because std::find returns an iterator, and nobody wants to look at long STL iterator type names. But if the next thing were this: auto foo = *it; ... I would be sad.
- RogerL 12y agoAre you sad because of the auto, or the terrible variable names? auto gps_position = *saved_position; I can think of cases where I'd want to see the type of auto, but not often.
- SamReidHughes 12y agoThere are a ton of times in C++ or any language where you want the type. If having the type makes the code more readable, you should have it.
- RogerL 12y agoThis is what everyone is saying. The only question is whether to ever use auto in non-generic code. I think it is unarguable that if you can write foo(goo()) in a clean way you can equally use auto. And if you can't do the former, you probably can't do the latter. Balance, proportion, judgment, and code reviews are how you get great code, not 'never' and 'always' rules (which your comment makes clear you agree with).
- AlisdairO 12y agoOne of the nice things about explicit types is that you can make sense of the code even when the person who wrote it wasn't the best at variable naming. I tend to distrust relying on convention. That said, presumably one solution for such an issue is that have IDEs that can easily tell you what the type of an auto variable is.
- maxlybbert 12y agoWhy? You know foo has the type of "whatever 'it' dereferences to." Why would spelling it out be an improvement? Would your opinion change in a templated function where the type is, itself, a placeholder (e.g., "typename T")? What if "it" were originally an unsigned int* but a refactoring changed that to a long*? Would you prefer the programmer hunt down all cases where "it" is dereferenced to change the type of the result, or would you prefer the programmer use "auto" to begin with?
- millstone 12y agoThe types of dereferenced iterators can be nasty. See for example std::map::iterator. Using an explicit type allows you to keep less in your head at once. It's good to give stuff names, and say what they are. In a template, you're somewhat better off using Container::value_type. Though not by much. The unsigned int* -> long* example is a good point. But auto doesn't fully solve that problem - consider something like iter = x or some_func(iter). The auto also makes it more work to figure out what the underlying type is. You're probably better off using a typedef.
- maxlybbert 12y agoYou're certainly always allowed to use a typedef. I can just tell you from my experience that auto is more than "a nice thing to have in very specific circumstances." For me, at least, it's "a nice thing to have in almost all circumstances, with a handful of exceptions." When in doubt, I type "auto."
- Scorponok 12y agoYou should only really use "auto" when the type is clear (like in your example #2). If it's not clear, you should probably specify the type so that it's more obvious what's going on. #1 and 3 are probably fine if x is defined right above y, but maybe not if they're at the bottom of a long function or similar.
- Iftheshoefits 12y agoLike anything else in a language in which the syntax and other features permit writing correct code that is difficult to maintain or decipher, your objection has to be handled by a human being (or well-designed analysis tool) at the "meta" level (e.g. by code style guidelines/policies). I agree "auto y = std::move(x)" is (likely) poor coding practice. I only use "auto" basically as a shorthand (e.g. instead of writing "hand_crampingly_long_container_iterator_type v = std::fn(c.begin(), ....)). I have mixed feelings about "polluting" the namespace with "useless" typedefs and similar aliases versus using "auto". The former leads to very explicit code, but lots of extra "overhead." On the other hand, "auto" is much more powerful than a convenient in-place alias as I've described, and I've rarely found myself in the position of looking at code and having to truly ponder over the type of an "auto" variable. On the other other hand, seeing a lot of either may indicate something else about the code and whether it ought to have a design review. It's (somewhat) subjective.
- RogerL 12y agoDo you allow this to pass code review (ignoring the bad name 'foo')? foo (A*x); 'A * x' creates a temporary with no documented type, but the conceit here is that we don't care. They could be ints, floats, matrices - any data type that implements '*'. By and large that is fine. Here you are just moving x to y. What is the type of y? the same type as x, which supposedly didn't cause you heartache throughout the rest of the function. Stroustrup gave an interesting talk this year about how, everytime he introduced a new feature, people complained that it was not clear and asked for very verbose syntax. Instead of foo<T>, we get template<typename T> foo<T>(T, well, you get the idea. Now everyone is familar with templates and loath the verboseness. Back to auto, it allows for 'generic' programming. Do you object to templated functions? template<class T> T add (T a, T b) { return a + b; } That seems really clear to me, as is the auto version: auto add (auto a, auto b) { return a + b; } If that is clear, why is auto x = a + b; unclear? I can certainly come up with counterexamples where it isn't clear, but by and large I love auto, and use it all the time. (counterexample: we care about the type of x because we need to cast to a 16-bit int because it is going to be use to communicate with hardware).
- plorkyeran 12y ago> we care about the type of x because we need to cast to a 16-bit int because it is going to be use to communicate with hardware IMO you should still use auto there and use a static_cast<int16_t> on the RHS to make it clear to the reader that you're deliberately converting it.
- RogerL 12y agoYou're right.
- yongjik 12y agoProblem arises when we find "y" deep in a function, searches for its definition, and encounters "auto y = std::move(x);". Now we have to know the type of x, and if x is defined similarly, up the chain. Not fun. foo(A * x) doesn't pose similar problem. On the other hand, if I write auto bar = foo(A*x); Then we can have the same problem (esp. if foo is a templated function).
- maxlybbert 12y agoWhile C++ programmers are still getting used to type inference, it's not really a new concept. It's been in other languages for decades. Personally, I avoided type inference in C# for a long time. But I never ran into a bug caused by type inference. While C#'s (and C++'s) type system isn't as advanced as Haskell's, if you screw up the types you generally get a compiler error. If "auto y = std::move(x)" compiles, then why do you care what type y is? You know it's the same type as x, and you can do anything with y that you could have done with x. There's really nothing to fear.
- millstone 12y agoConsider this simple code to swap the first two items of a vector: vector<T> foo = ... auto tmp = foo[0]; foo[0] = foo[1]; foo[1] = tmp; When T is int, it swaps as expected. But when T is bool, instead of swapping, it copies the second item over the first, with nary a warning. And I'll wager you could stare at that code all day without spotting the bug. So yes, be afraid.
- Dylan16807 12y agoI object to calling vector<bool> a vector. I'm afraid of it, not of auto.
- detrino 12y agoThis is a contrived example. Not only is vector<bool> the real problem, but this is a standout terrible way to swap elements, especially in generic code.
- millstone 12y agoYou have to admit, it's hilarious that the most natural swap implementation you could possibly write (and the one used in C++03) is now "standout terrible." It's not wrong to point that out, but it is wrong to assign blame to the hapless programmer instead of the language. And even if we make it not-generic, and add some C++11: std::vector<bool> foo = ...; auto tmp = std::move(foo[0]); foo[0] = std::move(foo[1]); foo[1] = std::move(tmp); We've fixed nothing. The problem remains! (It works if you use std::swap, I think because whatever type tmp resolves to happens to overload std::swap to do the right thing.) So why is vector<bool> the "real problem?" It's because its operator[] doesn't return a bool&, but instead some type "convertible to bool," that may do lots of other stuff too. But the standard is chock-full of language like that. For example, with iterators: `str.begin() != str.end()` Is that a bool? `str.begin()[2]`. Is that a char? The standard doesn't require either. Care to roll the dice by assigning one to auto? Type inference works well in other languages, but it is more dangerous in C++ due to the risk of implicit conversions. C++ does so much stuff for you under the hood that it can be quite hard to figure out what is really going on, and auto only makes that problem worse. Use with caution.
- 10098 12y agoI think it depends on context, I like using auto when the type is clear from the surrounding 5-7 lines of code or if it's for some nested-container-iterator stuff. The rule of thumb is, if the context doesn't provide enough information about types to deduce what the inferred type for `auto' will be after a quick glance, then don't use auto.
- wmkn 12y agoScott Meyers (the author of this presentation) makes a pretty good case in the following presentation: http://vimeo.com/97318797 http://vimeo.com/97318797 Unfortunately the slides are not online. But his presentation starts with a case of why 'auto' should often be preferred to explicit type declarations.
- EpicEng 12y ago"For example this seems ok to me, because the type is on the left" auto* x = new Foo(); Are you only referring to the fact that the type is noted at least once on that line? I ask because that would not pass my code review at all 99% of the time (manually allocating memory that way.) As for something like auto x = std::move(y); I couldn't say it is bad without seeing the context around it. Is it a small function? Is it obvious what y is? Looks fine to me in most cases.
- TheSoftwareGuy 12y agodo you NEVER allow dynamically allocated memory? or am I misunderstanding you?
- Nursie 12y agoIn modern c++ you can get away without it by using a combination of RAII and the various shared pointer and container features. It's an odd mental leap for this old C hack, but it works quite well when you get into it. Bonus - no 'free' or 'delete' necessary.
- TheSoftwareGuy 12y agoPlease correct me if I am wrong, but RAII isn't flexible enough to allow for lazy initialization. the various STL pointer types (std::shared_ptr etc..) are great, but they do incur overhead, and sometimes that is not acceptable in e.g. embedded systems.
- cottonseed 12y agoI think he means it should read something like: auto x = std::make_unique<Foo>(); Unless you're implementing an allocation policy, you shouldn't be calling new and delete directly.
- EpicEng 12y agoThere are very few instances in which manually managing memory in C++ is justified. When you have `unique_ptr`, `shared_ptr`, and all sorts of container classes, it simply doesn't come up in most cases. If you see operator new being used you should be suspicious.