Skip to main content
gamedev.net
🔒 Locked

c++11 iteration and warnings

Started by larspensjo 25 replies 4.9k views
Original Post
larspensjo
larspensjo

I want to iterate through an object in c++11, and like the new syntax. If I have the following code:


for (auto x : object) {
    ...
}

I have a case where I don't want to use 'x'. That will produce a warning, as expected. How do I best suppress this warning?

I suppose I can add a line (void)x; but maybe there is some better way?

If there is no better way, can I be sure that the void reference to 'x' generates no code?

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

Cornstalks
Cornstalks

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

I don't know why you're iterating without using the reference at all, but I'd do the following:


for (auto i = object.cbegin(); i != object.cend(); ++i) {
    ...
}

It shouldn't be any worse in performance than a range based for-each loop (I imagine it'd be exactly the same, with optimizations turned on).

As C++11 currently is, there's no way to do a range-based for loop without a range declaration.

swiftcoder
swiftcoder


I have a case where I don't want to use 'x'.

Can you elaborate on the use-case?

Apart from implementing your garden variety REPEAT 10; this seems a little suspect.

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
ApochPiQ
ApochPiQ
Hiding side effects behind iteration seems... gross to me.

It's more verbose, but I'd rather see your code look like this, just for sake of clarity:
struct MovementSystem : public System<MovementSystem> {
  void update(EntityManager &es, EventManager &events, double dt) override {
    for (auto& entity : es.entities_with_components<Position, Direction>()) {
      std::shared_ptr<Position> position(entity.get_component<Position>());
      std::shared_ptr<Direction> direction(entity.get_component<Direction>());

      position->x += direction->x * dt;
      position->y += direction->y * dt;
    }
  }
};
It took me several read-throughs of your code to figure out that it's equivalent to this. IMHO that's bad, because it means your system is not intuitive enough, or idiomatic in terms of how iteration typically works in C++.

It's also clearer that this code will optimize cleanly. Moreover, if you know you don't want to affect an object's lifetime during the update, you can opt for raw pointers instead of shared_ptr which gives you a little less overhead, assuming that that sort of micro-optimization is relevant.

That's just my opinion, though :-)
Ravyne
Ravyne

If you're not using x, its probably more efficient to iterate over an integer count with size of the number of objects.

throw table_exception("(? ???)? ? ???");
Stroppy Katamari
Stroppy Katamari

If there is no better way, can I be sure that the void reference to 'x' generates no code?

There is of course the old way of using iterators, but it looks much better with the new way of doing it. Or tell the compiler to ignore these warnings, which I do not want to do.

If you use "auto& x" or "const auto& x" as the loop variable, there are no copies.
And if I understand correctly, the standard guarantees (void)x; does nothing with x either:
http://stackoverflow.com/questions/689677/casting-unused-return-values-to-void
Stroppy Katamari
Stroppy Katamari

While all true, "auto& x" still may cause something to be done in the iteration which may not be needed/desired. I.e. de-referencing the iterator in order to get the reference initialized. At this point you have to look at assembly to figure out if the compiler is doing what you intend (simply dropping the item and optimizing it away) or if it is actually doing unused work behind your back. As this is a relatively new language feature, who knows if the compilers are really optimizing this specific case correctly, I don't trust compilers that much and prefer to be explicit.

I'd prefer to always write the code as nicely as the language allows and if it matters, verify the compiler is doing the right thing. That may be more of an idealist approach while yours is a more practical one. Considering that range-for and auto are probably the most-used features of the new standard, I think support will be pretty good. Lars' library isn't exactly shy of using C++11 to begin with.
Bregma
Bregma


always write the code as nicely as the language allows

I would assert that using conventional constructs to invoke hidden secret indirect subtle Klever nonmanifest behaviour is allowed by the language but could in no charitable way be described as 'nice'.

Stephen M. Webb
Professional Free Software Developer
larspensjo
larspensjo

It took me several read-throughs of your code to figure out that it's equivalent to this. IMHO that's bad, because it means your system is not intuitive enough, or idiomatic in terms of how iteration typically works in C++.

Thanks for the analysis, I can see you spent some effort doing it. And I agree your proposal is more readable and intuitive.

The link I used was to my fork, but it is not my system, though, just a library I am using.

Matias Goldberg
Matias Goldberg

The presence of 'auto' is becoming wide spread and is a really really bad practice. Because it's only good in very specific cases (templates) it's common presence in regular code makes me think you're following bad practices, which doesn't talk good about you.

In regular code, there is no benefit in using it (other than lazyness) and inherits all the problems from weakly typed languages: difficulty to grok the code for other programmers who are trying to understand what your program does or are trying to debug it. What is the variable supposed to hold? A string? an int? a float? A DOUBLE??? How do you find floating point accuracy errors when you think it's holding a double when it's actually holding a float?
A simple typo could make a gigantic difference (i.e. 1.2f vs 1.2, or '7' vs 7, "0xA" vs 0xA )* and what's worse, by using auto the coder is preventing the compiler from raising warnings (or even errors!) that would be catched instantly at compile-time if using explicit types.

Reading "MyGuiLib::TextElement* ptr = parent->getTitle()" tells a lot more than "auto *ptr = parent->getTitle()". Typing speed is not an excuse, we have intellisense for writting that. But using intellisense to understand what getX() returns while reading, is very distracting. Reading code is a lot harder than writting it. Even more if you weren't the one who wrote.
Furthermore, explicit types encourages to the one who is writting the code to be actually aware of the relations between each system (which leads to better design).

Unnecessary hindering of code is bad. In some languages it works well (i.e. Python) because they're designed to ease development are very high level, and particularly, Python makes it very hard to shoot yourself in the foot, something C++ makes very easy (harder than C though) because it's very low level.

The usage of auto is encouraged in specific scenarios, for example the cases where the creator of C++, Bjarne Stroustrup, talks about. His example is a very valid case of a good usage of auto. But abusing the keyword for everyday code? No thanks.

Makes me remember that I read in the 90's C++'s polymorphism was the new hype and some programmers wanted to make derived classes just for the sake of making them (and make every function virtual, btw) some reaching very deep parent relationships.

Just because there's a new cool feature everyone's talking about doesn't mean you have to use it.

*Edit: Just to clarify:

1.2f is a float, 1.2 is a double,

'7' is a char holding the ASCII value 0x37, 7 is an int holding the value 7,

"0xA" is a string, 0xA is a integer in hexadecimal.

Ravyne
Ravyne

Don't use auto in place of using regular type declarations, but other than that I don't think that the sky is falling quite so hard as Matias is worried about.

In general, use auto to:

  • avoid repeating yourself.
  • refer to an un-uterable type.
  • store something that really is dependent on the result of an expression.

You get yourself in trouble when you use auto to simply avoid thinking about types and their consequences.

Used appropriately, though, auto has worthwhile benefits, including a reduction of maintainance effort, simpifying template code, and reducing visual clutter.

throw table_exception("(? ???)? ? ???");
swiftcoder
swiftcoder

avoid repeating yourself.
refer to an un-uterable type.

Why not typedef it, like everyone else does? Same short declarations, no magic sauce to make reading difficult.

store something that really is dependent on the result of an expression.

This is really what auto is designed for. There are certain things that are very hard to efficiently compile without auto (for example, boost::spirit used to have awful trouble with recursive declarations).
Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
Ravyne
Ravyne


avoid repeating yourself.
refer to an un-uterable type.

Why not typedef it, like everyone else does? Same short declarations, no magic sauce to make reading difficult.

typedefs don't avoid the repetition in the cases I'm speaking of, if the type changes, you've got to change it in multiple places. auto solves these cases. typedef is great where it works, which is most cases. I'm thinking of stuff like 'std::pair p = std::make_pair..." -- not literal repetition, but just as useless. One could argue that this falls under the dependent type bucket, though.

And you can't typedef an un-uterable type, of course.

throw table_exception("(? ???)? ? ???");
swiftcoder
swiftcoder

I'm thinking of stuff like 'std::pair p = std::make_pair..." -- not literal repetition, but just as useless.

But std::pair p is almost as bad as auto. It conveys structure, sure, but it conveys nothing in the way of semantics. An explicit typedef std::pair Point is infinitely preferable to either (though a struct Point {int x, int y;} would be better still).

And while I'm sure that was merely a contrived example, I think the same holds for many common uses of auto.
Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
larspensjo
larspensjo

The presence of 'auto' is becoming wide spread and is a really really bad practice.

This is a most interesting topic, and I think we are going to see all kinds of practices of this (when to use and when not to use).

There is the recommendation not to repeat yourself, but then there is the problem for the reader of the source code to understand what is going on.

I used to favor the second view (and explictely use the type). But then I programmed Go a lot, and there is a similar mechanism. Assignments can be done as usual with "int a = f()" just as in C/C++, but there is also the syntax "a := f()" where you don't have to declare the type of a. This is used consequently everywhere, and it is recognized as a strength of Go (while still having static type testing by the compiler).

So my personal opinion is that I like to use "auto" for all but the primitive types. And I want the IDE to help me see what the type is.

Edit: spelling and wording.

blutzeit
blutzeit

The presence of 'auto' is becoming wide spread and is a really really bad practice. [...]

In regular code, there is no benefit in using it (other than lazyness) and inherits all the problems from weakly typed languages: [...]

I used to program a lot of C# and was of the same opinion regarding its auto-equivalent, the "var" keyword. Then I played around a bit with F# which completely changed my mind. F# is a strongly typed language but encourages you to not specify any types anywhere. It makes you have to write readable code without the help of type "tags" sprinkled thru out the code. It's a very good exercise. Going back to C# a completely refactored the codebase I worked with to use "var" for all variables except the "built in" types (in C# int, float, decimal, string are language specific aliases to System.Int32 etc). The code got shorter and much more readable.

Now, the type system in C++ is much more complex than C#, so I'm not advocating the same principle here. But I still think that liberal but purposeful use of "auto", when done right in the right context, can be a good coding style.

openwar  - the real-time tactical war-game platform
wintertime
wintertime

What I find weird about the new range for:

- There is the new cbegin and cend you should use on read access as to avoid the container preparing for a write, like for example a string making a private copy of the shared internal buffer.

- The range for seems to always use begin and end, but never cbegin or cend?

EWClay
EWClay

What I find weird about the new range for:
- There is the new cbegin and cend you should use on read access as to avoid the container preparing for a write, like for example a string making a private copy of the shared internal buffer.
- The range for seems to always use begin and end, but never cbegin or cend?


cbegin and cend are used to ensure that const iterators are obtained even if the container is not const. It isn't because non-const begin might do extra work; in fact string cannot use copy-on-write anymore in C++11. With range for, the iterator is hidden but you can declare the element const, which is sufficient.
Ravyne
Ravyne


I'm thinking of stuff like 'std::pair p = std::make_pair..." -- not literal repetition, but just as useless.

But std::pair p is almost as bad as auto. It conveys structure, sure, but it conveys nothing in the way of semantics. An explicit typedef std::pair Point is infinitely preferable to either (though a struct Point {int x, int y;} would be better still).

And while I'm sure that was merely a contrived example, I think the same holds for many common uses of auto.

I agree that a class, struct, or typedef is going to be available in many cases, but the semantics of my example would change along with it. Imagine instead:

Point p = Player.GetPos();

Is it terrible to have to write this? No. Is using auto instead going to lose tons of semantic value? No again, I argue. Each approach has its pros and cons, they just have to be weighed out.

Other times you find yourself needing "throw-away" or "temporal" types -- types that exist maybe in one place, and so don't need to be shared around. You can typedef those too, but I don't think that usually buys any more semantic understanding either. Most times that you use an iterator is a good example of this, lambdas are another. In functional languages, the pattern is to create these kinds of temporal types all the time wherever they're needed, and you hardly even think about it that way.

I do agree though, that over the next probably 2-5 years we'll see some real guidance about when its best to use auto or not. Right now we're at a level of "this is what they're designed for, this is what they can do.", which doesn't always turn out to be the same thing as "this is what you should do."

throw table_exception("(? ???)? ? ???");

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.