Descent3
Descent3 copied to clipboard
DescentDevelopers
Remove -fpermissive
#20 introduced "-fpermissive", a flag that turns non-conformant code errors into warnings, in order to be able to build on Linux.
We should manually correct these errors and disable the flag again.
Most of warnings is "taking address of rvalue", which can be fixed by using temporary variable that will be passed instead:
- ship_win.Create(&window, &UITextItem(""), 290, 50, 180, 140, 0);
+ auto t = UITextItem("");
+ ship_win.Create(&window, &t, 290, 50, 180, 140, 0);
Yeah, this only works because Create (or further calls down the line) actually makes a copy of the object passed, so we should definitely fix that & make that warning an error, probably.
nitpick: please don't use auto, it makes code unnecessarily hard to read
nitpick: please don't use
auto, it makes code unnecessarily hard to read
IMO auto is fine when the type is obvious.
auto t = UITextItem(""); is self-explanatory
auto t = getUserTagItem(); is not
It's not immediately obvious that UiTextItem("") is not a function call.
Also,
UITextItem t(""); even is shorter than
auto t = UITextItem("");
nitpick: please don't use
auto, it makes code unnecessarily hard to read. [..] shorter
To bikeshed a bit, auto x = ... when introducing a new variable makes it easier to read, instead of having a type present there (as long as it does not hide the type). Especially since a lot of other languages use var, let, val etc. in its place.
And naturally I am also a fan of the now possible auto foo() -> RetType { [..] } syntax! The relevant information, like function or variable name, is always present at a fixed offset and does not hide behind a std::unique_ptr<MyCustomType<T2, T3>> name_hidden_here :wink:
(But it is now all UITextItem item{...})
nitpick: please don't use
auto, it makes code unnecessarily hard to read
I have mixed feelings on that statement. I generally agree, but there are times where the types returned are templated (e.g. iterators on vectors) where the type is long and doesn't necessarily help. I agree though that typing:
int i = foo();
Is better than typing:
auto i = foo();
That's an exaggerated example, but you get the idea.
I have mixed feelings on that statement. I generally agree, but there are times where the types returned are templated (e.g. iterators on vectors) where the type is long and doesn't necessarily help.
I fully agree, auto does have some valid uses (another one: binding lambdas).
That's an exaggerated example, but you get the idea.
Not exaggerated at all, this is what many people do. I've seen C++ codebases that do this for every variable, even if it's literally just holding an int.
I'm sure some people like it (I mean, some people even enjoy dynamically typed languages like python that by default don't have any types mentioned anywhere except when creating an object of the type.. unless it comes from some kind of factory function of course), but IMO it makes code much harder to follow when you can't look at a variable definition to see what type it is, but must do another step to check what type the function (or whatever is assigned to the variable) returns.
If you think this is too much typing because you're using templated types that templated with other templated types etc, then maybe reconsider if your code really has to be this complicated - and if those heavy templating indeed is necessary (or at least genuinely useful), consider using a typedef..
I've read a book on modern C++ where the author advocated for using auto at literally every possible opportunity. I was pretty sure that was too far, but it does drive home that there is a diverse range of opinions on the subject.
I guess the recommendation I'd personally make is: use it if it makes the code easier to read; don't use it if it makes it harder to read. But I don't want to get preachy about it.
