I Rewrote This Entire Main File // Code Review

💰 Links to stuff I use:

Thumbnail facepalm icon created by Freepik - Flaticon.

[gratux]

Merging the destructor and Clean makes sense, since a destructor is meant to clean up, and do whatever needs to be done before an object is deleted.
However, I wouldn't expect a constructor to also start the game.

[Ba-gbbr]

StartGame should not be part of the constructor. The constructor should bring the object into an initial valid state by allocating resources, setting default values, etc. and nothing more. A constructor should not be used to run "business logic" of an object.

[sashakoshka]

bro literally made a constructor that blocks for the entire duration of the program

[clinsen]

The moment he put a function call in a constructor - I knew the shit's gonna go down in the comments 🤣

[Omsip]

The more logic there is in a constructor, the more ways it can fail. Constructors can't easily return a value, so you have to use exceptions to report a failure or store an error code that you can retrieve later or use a reference parameter to return an error code. Also, not too sure about it… but having an exception in a constructor might leave you with a partially initialized object

[kostiapereguda]

I disagree with starting the game in a constructor. If you put StartGame() into the constructor, you may as well put the Clean() into the constructor too. But then, the whole point of an object is gone, and it can just be a static function. So, I would leave the initialization in the constructor (adding the scene to the game), and let the user of the object explicitly control when he wants to start the game with a separate instance method

[kenarnarayaka]

I think the heap allocation comes from the fact that they're a C# or Java programmer, since they do the whole
shooterGame game = new shooterGame;

And are just used to using new

[gustavbw]

"Everything in Java has to be inside a class" - I've made it my personal mission to do exactly the opposite, and have been quite successful in storing functions in enums, annotations, maps ... just for some extra spice.

[jonnyrichards]

The "you don't need classes for everything" is such a good point. I just wrote a basic SDL engine and ended up using about 10 different singleton classes that all didn't *really* need to be classes at all - I just made them classes because I was coming from c# and that's just what you do over there.

[starup]

Idk about the constructor starting the game, that feels really odd, especially when you then change it to be stack allocated. Maybe it's because I don't really do a lot of C++, but when I read:

int main() {
ShooterGame game;
}

I really so not expect anything to "run". It's fine that it pushes the ShooterScene on the engines vector, so that it is, in fact, a "ShooterGame". But actually running the game from a line of code that to me basically looks like a variable declaration seems really weird and unintuitive for me (and therefore hard to read).

But personally I would probably just put it in directly in main anyway myself haha.

[angela_jx]

3:35 it actually is not the same, there can be a massive difference. When you use “default” or don’t define a destructor that means that the class is trivially destructible. However if you define the destructor EVEN if it’s empty the class is not trivially destructible anymore.

This can have quite a big impact on the compilers ability to optimize your code, it can affect how certain containers or templated types like std::optional will work with your class and much more like even the ability to memcpy the class.

Basically: follow the rule of 0. Don’t define a destructor if you don’t need to.

[Stdvwr]

This is such a typical code review it should go into a museum. Bike shedding a perfectly readable code, suggesting changes based on personal taste, never delving into the actual parts that define the functionality

[GreenFlame]

Man, loved the video!

Your code, style, approach and explanation are such a joy and truly evokes the most positive emotions. Thank you so much for what you do. I really miss working in a good company with good projects/codebase :)

[mysteriousdbs]

I completely agree that a class should not be used here. When I write code, the first thing I think of when it comes to the question of "should this be a class" is whether or not it is responsible for managing data. Will this object be managing data such as conditions or objects? If not, it is much more suitable to make functions to perform the task.

Although, if the logic performed in a function is only a few lines of code like it is here, and the code is not intended to be repeatedly executed, I would just write the code where the function would be called with a comment above it to specify its role.

Avoiding new and delete will probably be a debate lasting to the end of humanity... I know, "who asked for my opinion?", but I'd say avoid using it unless the object needs to be accessed outside of the scope, such as initializing a new object then returning a pointer (like a factory, in which case, the class that made the object should keep a record and delete it when required, lifting the burden of memory management from the caller).

I still need to learn smart pointers, it looks like they would really help with memory management.

Side note (and thanks if you're still reading), I see a lot of people complaining about starting the game inside the constructor. This really depends on the purpose of the method that starts the game, as Cherno states, it should be 'immediately terminated', and I agree. Starting the game should do nothing more than flip some application state which says that the game should now run - this takes almost 0 process time, and doing this in the constructor of this pointless class serves only to make the code cleaner.

Realistically (and without a class), the method to run the game should just be called immediately after intialization and loading the game, as I cannot possibly see a reason why a run method would be called without the intention of starting the game.

I need to edit again! Forgot to say this was a great video, people coding in C++ (including myself, I am not perfect at this by any means) need to learn the semantics of these core C++ constructs, and even if you're an experienced coder, I've been coding C++ since I was 11, we still can easily mess this up - self-review is important, never write code once, see if it works, then leave it: what you wrote may have a much better solution you didn't see at the time.

[not_ever]

If you create a user defined destructor (or = default), you should be following the rule of five, which is a really great incentive to not use a class if you don't have to.

[birdegop]

I enjoyed this video much more than previous ones. This one is just straight forward, precise, no out of context talking and opinions.

[betterfly]

That "I don't wanna talk about it" at the end sounded just right lol

[hansdietrich]

The constructor starting the game is not good very self explanatory

[WhoLeb]

14:40 I am trying to wrap my head around the audio engine in hazel and there are a bunch of vectors with raw pointers to SoundObjects. They might be RefCounted though, I don’t really remember

[SuperiorZeeko]

Your code reviews are really fun to watch