Watch the rest of the Code Review for this project here: ukposts.info/have/v-deo/emahgW-Cmn93pmg.html

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.

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.

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

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

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

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

The constructor starting the game is not good very self explanatory

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.

Watching this makes me happy that I learnt Rust in the past year. It just automatically makes you aware of all these issues and when you get the hang of it writing C++ will basically be a piece of cake!

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 :)

Your code reviews are really fun to watch

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

This is so valuable, I like the way how you talk and optimize it without any hesitation. Being direct and strict about practices and code style is a good thing IMO. Especially if you can argue why it is good/better. Would love to see more of this series

Please make more videos on this code review. This video was very informative. I'd like to see your approach to the replacement for the parameter of AddScene()

Yeah, it's really interesting to hear your thoughts on creating a pointer in a function call. Please make the next part

I love these videos, and I'm not a beginner, I''ve created my own game engines, other softwares, etc. But it's great to watch these videos and trying to predict what you're gonna say. It's kind of like a game trying to reach the end predicting everything you're gonna say, sometimes we forget something and we think "Oh God how could I've forgotten this?". Please continue to do these videos, they are great. :) And yeah, it's very irritating when someone uses classes and heap memory for everything, in C++ we're really free, my favorite language. PS: You should have the unofficial C++ mascot plush there, it would look great in the videos, as sometimes you review some pretty unhealthy code and the mascot would fit perfectly, and you could exchange that mascot by a healthy rat plush at the end to show how the code got healthier. :)

Please continue reviewing this code! This type of video is very helpful for beginners

i love these style of videos, please do more :))

"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.

Would love to see a continuation on this!

I learnt a lot! thanks for uploading

I am learning lots. Thanks.

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

Great video! Always too short. What people need to extract from this video is that you want to simplify the way you solve problems. Because sure, in a simple file, all of this seems irrelevant, but if you add up bad practice upon bad practice in hundreds of files, this becomes a nightmare.

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.

As a linux user I was today years old when i learned that windows has a distinction between "console" and "windows" apps lmao 🤡

I love seeing classes like these get crammed into a single function. It's just so satisfying!

I really like static create functions where you do heap allocation for the object and then a destroy method that cleans up and deletes

Great review

I enjoyed this, I'm glad you pointed out the law of demeter violation. Im also not a big fan that you have to add a scene before calling StartGame. If the engine requires an initial scene(s) it should require it via constructor instead of using temporal coupling.

The last thing about handling pointers, that's a cliffhanger, would love to see a video on that as well. Guess that'd cover ownership aspect

Love this!

I might still not know anything but this video really useful so I can organize better, thank you

8:33 On certain platforms it's not true, like microcontrollers, where there is no operating system, so it's better to clean up explicitly for portability

These videos are very useful for learning C++.

In regards to logic in constructor/destructors, you do you boo, but beware that calling virtual functions of the object in its constructor/destructor, it will not call any overridden versions of said functions due to the derived type not yet having been created, or in the case of a destructor, it's already been destroyed. Just one of those fun little quirks of C++ worth thinking of when adding logic to constructors/destructors.

Return values in main might have implications for scripts used by build farms to check if the runtime terminated without error . I think I also have seen compiler warnings in cases where functions are declared to return a value but don't. Maybe this is ignored for main().

Would love to see a video about pointers as you suggested at the end.

Actually, since Java 21 it's not necessary to write the whole "public static void" in "public static void main" one can just get away with writing "void main"

What visual studio theme are you using man? It looks amazing

I think the only change I would add on top of what you suggested is to just have the constructor initialize the engine and require calling a function to actually start something running. Otherwise, I agree with all the changes you suggested.

That was useful, thanks

I feel like you should record a "standard disclaimer" and link it at the beginning of every CR - something along the lines of: "If you aren't a trained programmer and you've managed to write an application that works, it's already an amazing deed and you already are an amazing human being. That said, code reviewing is a procedure coming from enterprise environments that's supposed to bring up your code on par with the company standard or, if you're lucky to have such talented peers, to help you progress as a developer to an even higher standard, therefore, it is mostly criticism, but given in good faith and without any prejudice. It's not supposed to put you down - it's supposed to widen your thinking horizon and to make you both: more understanding of the inner workings of your code and better at seeing some issues coming at you from far away, so that you could avoid them early and save yourself a headache at later point". Because, man, we do get off-putting to the newbies when presenting our code religion. 😆

More of these pls

I want more code review series videos

great one!

I always thought that performing significant class functionality inside the constructor was, if not technically problematic, perhaps a bit obscure. It also doesn't feel "self-documenting" to any reviewer because they're not expecting the constructor to be the main thread of execution. Maybe I'm just old-school, but I think the stack construction, followed by game.run() in the main function would be relatively clean and moderately self-descriptive as to intent.

Releasing resources has more implications than just memory; if you're holding onto file handles, create temp files that need to be cleaned up, etc then it does matter whether or not you're explicitly destroying those. I think it's better to encourage clean-up hygiene than to handwave it as unneeded even if it's generally a non-issue most of the time... similar to multithreading, most of the time it'll work fine, but then that one edge case that could have been prevented by proper coding hygiene turns into a month long debug-fest. The other comments on error handling & control flow being difficult in constructors is also good feedback, though I think in this case (class acting as entry point for the game) it's a non-issue. Probably worth a note on when it's preferable to use initializers vs constructors when doing this type of refactor.

i never get the reason for deallocating memory (cleanup) RIGHT before your program exits. just exit the program without cleanup, this is not only operating systems job, this also makes your program exit faster edit: nvm he says this in the video

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

13:50 I immediately know that Cherno would not like Zig because of this reason. I would argue that in certain performance situation the caller should be aware of what the underlying data structures they are using (but of course not in the situation shown in the video.)

5:10, this makes sense if the project is too short and simple. But in games we have the new game option, which is better to have a f() for that, instead of constructor, which would demand to rebuild the entire object.

Yes, "pause" command doesn't exist on Linux

Ocd hitting cherno hard😂🤣 (pls bring back ray tracing series)

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.

1:31 as much as I know it is used on Windows only. For Linux you have to use wait -p or something like that

Thank you for today's stream, I enjoyed CLion all the way)

Ah, the poison that is the "OO mindset."

Nice! 👍

It does matter what you return from main. The return from main is the exit code, and if you omit return 0, it does not just do a return default(int) or whatever, it does not return at all - this is UB.

Everyone arguing if the function call should be in the constructor or not is missing the obvious. The ShootingGame class shouldn't even exist. It's just a wrapper for the engine object. There's no need to even have a class there. You just have a non-member function such push_back the ShooterScene, StartGame() and then Clean(). That would dramatically clean up the code. Edit: ff to the end that's what he ended up doing. So why are people in the comments arguing about this intermediate change? Did they not make it to the end of the video?

I once began turning a console project into a Windows project. I switched main to wWinMain during the transfer process, and just to make sure things were working correctly, I ran the project. It compiled nice and fine, but as soon as it ran, my antivirus flagged the executable as suspicious and prevented it from running. Once I changed it to actually spawn a window, the antivirus was okay with everything going forward. I suspect that antivirus software treats any software that uses WinMain but doesn't spawn a window as a virus.

I could watch refactoring for hours

I do think it can make sense to have it in a class, obviously not if your main function literally just creates the object and then does nothing else with it and then the main function ends like in this case, but if you wanted to expose this class for use in other programs, they might wanna manage the state of one game instance by having a game variable and then start and end (or restart, pause etc..) it when they like, might wanna make multiple instances, etc. And I don't think the constructor should automatically start the game.

I wouldn't start in the constructor, but I'm not dogmatically against it.

Others have already provided valuable insight regarding the logic behind why constructors shouln't handle business logic. I'd add that there is no reason to use hungarian notation. It is redundant.

There's nothing better than seeing Cherno shit on code that you're guilty of yourself. Incredibly informative as always and thank you to the person who contributed.

Cherno goes OCD - A documentary 😂

You should not start a game from within a constructor. The constructor should be used for initialization-related stuff. If there is an exception thrown from the constructor, you have to clean them there as well, as there will be no object to call its destructor later. And the delete method will fail.

What theme do you use can you tel me please

15:59 that's good idea!

Hi Cherno and everyone! I want to buy a new laptop for programming... what would you recommend, a windows laptop with an intel processor and nvidia graphics card or a macbook pro with m3 chip? I want something versatile and powerful to build different kinds of apps

An app set with subsystem Console doesn't even need system pause, VS will automatically make the window persist, but people keep writing system pause for some reason. It baffles me.

This is why IMO being mind locked deep into OOP is not good, it conditions people minds into thinking everything should be a class. And this many times causes code to be way more complex, harder to read and at times way slower. Doing pure functions has its problems but it also has advantages and IMO OOP C++ programmers, shouldn't be afraid of using some C functional programming ideas.

All good, but I would not stack allocate Engine. A non trivial engine might be bigger in memory than the tiny stack.

Hey Cherno. Im working on my own 2D game engine which uses some cool libraries. I would love you to review my engine when it’s done!

Constructors and destructors aren't a benefit of OOP, they're a benefit of the known lifetimes in C++. You can't use RAII in, for example, Python, because objects are cleaned up whenever the runtime feels like it, and you need to use a context manager instead.

I actually prefer the classless approach for some tasks even in language like C# and Java. It's just cleaner and less complicated in the end

Is it even necessary to keep the clean function on the Engine? probably the destructor would just fit equally here as before

Few people would be interested about non gaming country turning into gaming country with a very first shot missile, but it's about to be discussed.

If the constructor does not start the game you can use the class in tests much easier without mocking. I would not expect this behavior of starting the game. However I don’t see the need for a game class…

Deletes Clean to make code cleaner. Commit message: Cleaned Clean

Do we want more videos like this? How come a question like this even exists? :)

I hope some of my Jr engineers give this a look.

how the semantics is in my engine, is a raw pointer means that it is not owned. i use std::unique_ptr when it is owned.

if you remove return 0 from main, does it always return 0 or what does the program return?

Can I send you vulkan hardware ray tracer for code review?

This "don't tell the c++ community" is so funny!

what color scheme is this?

Excellent, excellent, excellent.

game->StartGame and game->Clean looks like something a C developer who is learning C++ would do because in C you have to manually call constructors and destructors lol

Cherno Code Review Cheat Sheet: - See "new"; - Rewrite file; - Don't look at anything else.

But there's something you're missing when you start the game from a constructor. A constructor has no return type and if you wanted to return an error, you'd need to throw and catch an exception. Using a function would allow you to return an error code. In this code it didn't matter as there's no return code, but still. It's a valid reason for not doing stuff in a constructor when you should be avoiding exceptions. Also since it's only a pointer to the constructed class, wouldn't unique_ptr be better?

Does anybody know what color theme cherno is using?

Can U make a code review of Godot?

psv main in new java can be outside of class

What ide is he using?

Yes Please Expand on New.

interestibng