C++ Code Smells – Jason Turner



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.

Save the date for NDC TechTown 2020 (31st of August – 3rd of September)

Check out more of our talks at:
https://ndctechtown.com/
https://www.ndcconferences.com/

35 thoughts on “C++ Code Smells – Jason Turner

  1. 35:25 A good reason for extern const could be a constant whose value you don't want to expose in your header, so that later you are able to change the value in your library without breaking ABI. I consider it a rare but valid use case, because ABI compatibility promises are rare in the C++ world.

  2. Wait so why use const auto if you can specifically declare it. Also, I feel the use of constants a bit excessive. What’s the point of having all your variables immutable?

    Moreover, comments seem a little sparse.

  3. The code at 17:44 got a int value which should be double, assumes that the number of hoses ist the same like pipes because in line 12 the hose radius is multiplied with the pipes radius. Should this be like that? Strange code!

  4. 42:45 why does he despise the std::endl on line 14? Sending n to std::cout isn’t sufficient to guarantee that the stream is flushed correctly (although in this case the program terminates shortly after so it won’t matter).

  5. I use extern const on values that need to be used in many places and that may need to be tweaked now and then. Oftentimes, this includes UI element colors, dimensions, etc. If you put them in a .cpp instead of a .h, when you need to tweak one, you only have to recompile that one file. If it was a constexpr in the header, you would have to recompile any files that include that header directly or indirectly. I've worked on some large projects where changing one header can cause a 5-10 minute rebuild. Other than that, I see no reason to use it. I don't generally use accessors on constants like these.

  6. Int-overflow… had recently discovered a major bug with that – multiplication of 2-4 ints that each have typical values between 10 and 1000, but up to 99 999. But due to company policies and the shear size of the codebase…. well, the calculation stayed based on 32-bit signed Int but i just added 2 sanity-checks. The first is just doing the same multiplication but with floating points and then checking if the result is significantly different – if it is, redo the calc but with a subset of the requirements – if it still overflows it is a fallback to a small but fixed value.

    In the whole software nobody cared to check for the range. User-input numbers are taken without any real sanitization and then stored as signed ints.
    Funny if for example the expected input is say a number between 0 and 24, the code checks the input against those bounds and then sends it to the DB. Just that the user can also input 2147483653, the check says its ok as it stored it with 32 bits for the comparison, but sends the "correct" input to the next process.

    And seriously, we are not running on some small embedded systems, we do not use ANY instruction-extensions like SSE (They are disabled in the compiler-flags), we are not memory-bound. There is no reason or benefit to using 32bit values for nearly everything. But it often is infuriating as most of the values can be reasoned to not exceed the 64bit limit ever and any input-value bigger than the 32bit limit should not be allowed.

    const_cast … have to use that a lot. Many classes (specially things like maps) are auto-generated and only offer const-access and no way of declaring anything mutable.

  7. having a factorial calculation function that stores the factorial in a 32 bit integer? the max value it can calculate the factorial of is 12! signed or unsigned.

  8. Some please help with this.

    A function called eta( ) which takes a distance in miles and a speed in MPH and returns a string that displays the hours and minutes until arrival. So a function call to eta(80,55) should return the string "1 Hour 27 Minutes".

  9. The decomposing multi-step functions into lambdas is needlessly complex and far less readable than a simple multi-step function, when dealing with other people's code I dont wanna spend the time figuring out what "clever" solution they came up with to express intent when they could have just written a decent comment that allows me to know what the intention was and then linearly parse what's going on to find the bug, for something as simple as finding the total area of a list of objects, I dont wanna deal with the standard library if all it's doing is adding complexity for the sake of being "clever".

  10. why is it always the case that the worst talks are the ones that beg for or try to force audience engagement? 😂 “interrupt me as much as possible” = “my talk is not engaging so you do the talking”. This talk is a engineering smell

  11. I use out variables to fill in strings where some parsing may fail so that I can return a bool success/failure code. Seems very wrong to write something like if (someStringParsingFunc(some_string) == " ");

  12. at 17:19, it would have been better to add all of the values for all of the radius, then calculate (value^2)*M_PI.. That way you are only using one math operation.. btw, the other problem with this code is that it really should have been a double float value for precision for the result within the current code because with larger amounts of doors and hoses, the code would end up becoming exponentially less accurate due to the lack of precision.

  13. I get that C++ can be extremely quick & portable, however this is a perfect demonstration of why C++ is slow and costly to develop…and this talk didn't even touch upon memory allocation and deallocation.

Leave a Reply

Your email address will not be published. Required fields are marked *