Understanding Code You Didn't Write // Code Review

CHAPTERS
0:00 - Resuming from last time
1:20 - Reading code you didn't write
8:27 - Easily find the entry point
10:51 - How the program works
15:32 - Code should be self-documenting ✔
17:14 - Comments in code to control tools ❌
17:55 - static_cast and dynamic_cast
20:19 - structs vs classes: when to use what
24:19 - Why prefixing member variables is useful

This video is sponsored by Brilliant.

#CodeReview

[roxasora]

I dont code much so I have no idea what you're all talking about, but Stowy is my friend and I'm very proud of him and his work being celebrated and featured here <3 Nice video

[Winterdev]

Thanks for the shout out! When I first moved from C# to C++ my first project was to make a game and I used your tutorials to create the first versions of the engine. If you look into the repo you can see traces of your layer model :) I don't know why that physics engine video blew up, but it's awesome to see it come full circle!

[Stowy]

Thanks a lot for looking at my code again ! These feedbacks and tips definetly where super usefull ! (although i'm very nervous that so many people sees it haha)
I'll try to fix the problem with the header files and the wrong usage of structs tonight, so if you want you can pull my changes before an (hypothetical ?) part 3.

[muumitramm]

Understanding Code You Didn't Write. Next day realizing it still was me.

[DesignCell]

+1 for self documenting code! Comments can lie when not updated but code never lies.

[fenril]

C++ style casts are encouraged because they make explicit exactly which casting behavior is intended. There are inherent benefits to using them over C-Style casts, especially on projects which have multiple developers touching the code base.

There are many cases in which complicated runtime casting can be vague if you are not explicit such as: (std::byte*)&integers[index] where integers is some container like an std::array<std::uint64_t, N>. The above is NOT a static cast and is a reinterpret cast which can be a very error prone cast if you don't know exactly what you are doing. The benefit of explicitly using reinterpret_cast<std::byte*>(&integers[offset]), is that you are "self-documenting". You might see such casting in low level optimizations when you're working on something like Serialization code to translate a variety of data types into and from a block of raw memory.

This tells anyone looking at your code: hey this is special cast we are treating the value at this memory address as a different data type that it normally is not meant to be treated as! Especially if you work on a large code base where you may have multiple authors potentially contributing to code this sort of explicit casting is preferable, because it is less error prone and tells other people "pay attention to what is actually going on here and if you don't know what this is explicitly doing go look it up". This can help prevent bugs being introduced by some other contributing developer.

Similar cases are common with the use of dynamic_cast. We want to clearly express "Hey we know because of some condition check or other control flow that this variable should be a derived type here at this point in the code." It also makes clear to any other developer where potential performance bottlenecks might be located at because they can see this is a run time polymorphic cast, if we are doing a lot of function calls off this we are probably doing a lot of vtable lookups here in this area.

Thirdly, searching through a codebase for "dynamic_cast<" instantly finds you all instances of runtime polymorphic casts (as long as the cpp convention is being observed) whereas it is much harder to get this kind of accuracy for solving performance, bug, or refactoring issues with the c-style casting.


Also, very much disagree about hating the C++ 20 auto ranged for loop features. They are very useful, especially for situations exactly like this one shown in the video, where you want to efficiently iterate over only the values of some non-contiguous node container like std::unordered_map (which I would assume _bodies is something like that).

Instead of having to write:

for(auto const& body : _bodies)
{
body.second->do_something();
}

all over the place or the other alternative

for(auto const& [key, body]: _bodies)

You can just use a view here to make explicit (and also prevent potential errors) that you only want to iterate through the values of this container. This makes it more clear what you are iterating over, how you are accessing the data (i.e. it's a view of the data therefore it is not being altered within this loop), and also doesn't expose the key value unnecessarily. Of course, this is just one of the many helpful uses of C++20 views, the purpose of which is to increase expression of intent when iterating or applying algorithms to data sets, and to eliminate a lot of the ugly boilerplate code needed to use some of the standard (or custom) algorithms with containers.

Personally, I would use a namespace alias whenever I use std::views, so that it's not so cluttered like: namespace sv = std::views;

[debmalyamitra]

THAT'S THE SMOOTHEST TRANSITION INTO A SPONSOR THAT I HAVE SEEN!! DAMN

[talhaibnemahmud]

Definitely a 3rd part needed. I just love this guys code quality 😍

[paligamy]

@2:38 Header files are compiled. They may be compiled multiple times and the reason why your linker doesn't freak out and start going all one definition rule on you is because they are exactly the same information.

@5:00 Stepping is the way to go. In OOP languages you can get burned ctrl+click'ing through projects because the IDE may not be smart enough to find what functions are being overridden especially when dependency inversion with interfaces is involved. Super annoying but part of the experience. Also note that in C++ when looking through a desktop application, main can also take a 3rd argument that shows the environment variables.
"A very common implementation-defined form of main() has a third argument (in addition to argc and argv), of type char**, pointing at an array of pointers to the execution environment variables.'

[thesupercomputer]

Absolutely agree on the Struct theme.
For me structs are data, maybe with functions to accept ony specific data, but still data.
classes on the other hande are there to manipulate data.

So in my code I can clearly distinguish between the the two and easily identify whether or not data could be changed by it.

[johnlim]

If not mistaken for the headers to show up in vs projects as source when using cmake, the header files need to be added to the target as a "source"; I.e. add_library(target header.hpp)

[andreyprotsenko]

I believe that having a single underscore and then a capital letter is prohibited by the standard, otherwise it's implementation dependent. Microsoft reserves double underscores, but also utilizes the _A pattern (std::array for instance has its only member called _M_elems afaik, which allows for some magic)

[spudgossie]

I agree with you on the struct vs class. The class keyword hints that C++ happens here where a struct would normally be a POD or something that allows passing to C code

[crystalferrai]

19:20 It is usually a good idea to add a virtual destructor to any class that is intended to be a base class. Otherwise you may end up in some bad situations where destructors and not called properly. As a side effect, this will allow dynamic_cast to function (assuming RTTI is enabled) because there is now a vtable. As a side note, RTTI adds overhead that is generally not desirable in game engine code, so it is often disabled and dynamic_cast will not function anyway.

[Forjugadname]

This is really great I didn't think about actually running the code in debug mode and watching it line by line.

[on-hvco]

I tend to agree with the separation between struct and class. When i think struct it's something that gets operated on like a data STRUCTure. Classes on the other and are the operators that work on the structures.

[fredygutierrez]

This is the exact reason I came upon your youtube channel. Trying to figure out my co-worker's 20 years old C++ code.

[enigma]

"Understanding code you didn't write!" My biggest gripe when dumped with a project someone else abandoned. And then you go on a call and seniors suddenly think you know how it all works.

[AntonioNoack]

The tricks of stepping through the debugger was really helpful :)
It lets me understand more why some people defend it over just printing things to the console :).

[nullplan]

The rules for the underscores are not different per compiler. They are indeed standardized, and they are the same for C and C++, for header file compatibility. In C11, this is all defined in §7.1.3. And it reads: "All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use. All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces." In this case, having member variables with one underscore and a *lowercase* letter is OK, because those identifiers are not on file scope. Change it to uppercase and you fall into the first category, where it is always reserved.

The reason it is always reserved is because the implementation may end up having to define internal macros to do its job, and if you use the same name, you are going to have a bad time.