C++ Code Smells - Jason Turner - CppCon 2019

—
—
—
There are a lot of rules to remember for writing good C++. Which features to use? Which to avoid? The C++ Core Guidelines would be over 500 pages long if you were to try to print it! What happens if we swap this around and instead of Best Practices look at Code Smells. Coding decisions that should make you think twice and reconsider what you are doing.

We will ask:

* What are the most important code smells?
* Does it simplify the way we write code?
—
Jason Turner
Developer, Trainer, Speaker
Greater Denver Area

I'm available for contracting and onsite training.
—

*-----*
*-----*

[Yupppi]

I love it when the presenter says "sorry" when someone talks at the same time during their presentation :D

[RedOrav]

I really respect Jason but I wonder just how did we get to a point where a simple for loop and adding comments to your code is a code smell, but adding a std::accumulate function with a lambda is the best practice to add some numbers up. Then you get the committee adding a spaceship operator which requires adding an extra header, that is a definite code smell like the initializer_list header or requiring tuples for structured bindings. Someone's gotta love it for sure... they make arguably useful features much more complicated and inflexible than they need to be. I guess I shouldn't be surprised at this point

[joycesmith]

15:05 Nice. The only thing remain is to have a member named total_area and an input parameter named total_area.

[von_nobody]

Last slide around 56:34 if there is no values in vector you get UB because difference cant fit `int`, even more `-min_int` is UB, `unsigned int` could avoid this problem but first you need correctly convert `int` to `unsigned`. Some thing like `((unsigned)i) + INT_MAX + 1`, this will preserve relative order of positive and negative numbers.

[MrZapper]

There are great speakers at cppcon but Jason has to be the best

[kyoai]

10:36, Line 12 : I think omitting "std::" is a code smell. Just by reading the code it is not clear if "all_of", "begin" and "end" is from the std:: namespace or some custom implementation, like boost. This becomes even more confusing considering in line 4 and 6 std:: namespace is not omitted, so writing std:: in line 4 and 6 but omitting it in line 12 may suggest that line 12 uses something other than std::. Unless the namespace name is unreasonably long or is an unreasonably deeply nested namespace i think namespaces shouldn't be omitted.

[rinkaenbyou]

31:11 Wow, I actually had this case where I had a constexpr complex state machine but I did not make it static. It became a performance hazard.

[alexandrustefanmiron]

I am actually leaning towards imposing out argument as a rule with reference return and optional output error argument and no side effects! Only things to reason about are already in the possession of the caller.

[sergeikrainov]

42:57 - what's the point of constexpr factorial? We don't know input at compile time, right?

[tomtanner]

Something that was touched on once in the talk - I find overlarge variable scopes to be a code smell all by themselves. The compiler might be able to work things out (possibly) but the reader of the code (not to mention the writer) likely won't be able to. And re-using a variable can result in some surprises.

[Don-jtch]

Would he ever regret asking for questions mid presentation.

[josee.ramirez]

Why is 13:49 better than 13:34?, anyone understands the "smelly code" (not only C++ people) and is only a couple lines longer. I am seriously asking the reasons why this is smelly. Seems like "lack of c++ features is smeally code"

[sinom]

in c that would have been an undefined nhuber of parameters because there you need (void). So they were probably thinking about that

[kered]

I'm going to need a whole talk on what the problem with const member variables is, because I don't understand. I use them many times when I have variables (especially pointers or references, but other types as well) that will never be modified even if the object itself is not const.

[code.sculptor]

Another code smell on slides 6.x: use of a macro that isn't even in the C++ standard. M_PI, although very common, isn't actually part of the standard. Worse yet, it's a macro.

[zvxcvxcz]

If a step is only 3 lines long then I'm not convinced it is a step and probably shouldn't be a function. It's just parceling and hiding code in a way that makes it less readable.

[zvxcvxcz]

26:08 Wait? That's really undefined? I guess it is, but probably only because it is difficult for the compiler to totally rule out indirect attempts at modification.

[NotherPleb]

Cpp bros finding out functional programming immutability is good

[cartesius]

Nice talk but sometimes making the code harder to read and more obfuscated just by using "modern" C++ features is definitely not the right way.

[PiotrWitoldNycz]

Am I the only one who thinks that writing "double area(const double)" is worst than simple "double area(double)". All these const next to trivial-type params are const-over-correctness (IMHO). I do not know/heard of even single real life example when such const helped in preventing errors or increasing performance. But I know examples when writing "template <T, Y> T convert(const U)" needed to be simplified to version w/o const because of types like "std::uniqiue_ptr". Moreover - in C++ world "foo(const int)" and "foo(int)" are exactly the same function signatures.