I Rewrote This Entire Main File // Code Review

  Рет қаралды 177,117

The Cherno

The Cherno

Күн бұрын

Пікірлер: 323
@TheCherno
@TheCherno 7 ай бұрын
Watch the rest of the Code Review for this project here: kzbin.info/www/bejne/eGfTfmyBmK55e5Y
@abacaabaca8131
@abacaabaca8131 7 ай бұрын
In terms of performance, is the function runs faster if it's a bare function, rather than a function embedded in a class ? If you use class maybe it's little bit slower because you later need to create a pointer for it. In terms of size is it the same memory size usage or not. But, I think it's the same.
@abacaabaca8131
@abacaabaca8131 7 ай бұрын
Can you share some use cases of `std::shared_pointer`. I've seen much of `std::unique_ptr` as of now but shared pointer is kinda of rare use case.
@__christopher__
@__christopher__ 5 ай бұрын
@@abacaabaca8131 Unless it's a virtual function, the function call should not be noticeably slower. It basically gets an extra this pointer argument. Also note that a pointer contains just the address of the object, so that is pretty cheap to obtain; for global/static and stack objects, you don't even have to touch memory to obtain it, as the address of a global/static variable is known at compile time, and the address of the stack variable is calculated from the frame pointer, which is in a register. Passing the this pointer to the member function also usually won't cause any extra memory access, as most calling conventions would pass it in registers. The only time you'd get an extra memory access because of it is if you have just so many arguments to the function that the extra this pointer makes it run out of registers and pushes one argument to the stack. But in that case, I'd argue that the function is likely so complex that the extra memory access doesn't matter (and also, the function probably needs some refactoring anyway). Also, in the case here, the member functions were all inline, and therefore wouldn't even show up in the compiled code (unless you don't optimize). Note that if the compiler knows the static type of the object, even a virtual function call can be fast, as the compiler can optimize away the virtual function call mechanism for that call, as the result is already known.
@gratux
@gratux 7 ай бұрын
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.
@zaftnotameni
@zaftnotameni 7 ай бұрын
yeah that also made me pause... constructor doing non-initialization related things (like run a whole game) sounds like an extremely surprising behavior
@nextlifeonearth
@nextlifeonearth 7 ай бұрын
Depends. It's more common to have things start on construction (ie: std::thread) but those usually don't block. game.run(); makes more sense (and imo thread.run(); also makes more sense).
@frango_molhado
@frango_molhado 7 ай бұрын
Totally agree
@abacaabaca8131
@abacaabaca8131 7 ай бұрын
Why you cannot do that ? If you have several member method that needs to be called in a specific order, why bother the hassle calling those methods after instantiation of the class. I've seen someone on KZbin doing the same thing i.e calling member method in constructor albeit it is in python. Is there any critical error / side effects with that approach ? Unless the member method/s that is supposed to be called in constructor will return a value, then you have no option but to call it later (after instantiation).
@gratux
@gratux 7 ай бұрын
@@abacaabaca8131 I would expect a constructor to finish very quickly, not block. For calling multiple things in a specific order, wrap them in another function/method.
@Ba-gb4br
@Ba-gb4br 7 ай бұрын
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.
@kukukudoes458
@kukukudoes458 7 ай бұрын
I would say it’s contextual I have seen a lot of popular application calling business functions on constructor. It usually calls some function that jumpstarts some specific business logic
@edjemonkeys4896
@edjemonkeys4896 7 ай бұрын
I completely agree. The pattern he ended up with read as if you were creating an object and doing nothing with it, not starting a blocking game loop.
@obinator9065
@obinator9065 7 ай бұрын
yeah, i have that and then I call a run method to start the game
@Silencer1337
@Silencer1337 7 ай бұрын
I think it was more intended as a gateway to show that you can discard the entire class, which he did.
@skeleton_craftGaming
@skeleton_craftGaming 7 ай бұрын
​@@edjemonkeys4896I think that was exactly the point, this is a lesson in the fact that you don't need objects everywhere.
@sashakoshka
@sashakoshka 5 ай бұрын
bro literally made a constructor that blocks for the entire duration of the program
@clinsen8576
@clinsen8576 7 ай бұрын
The moment he put a function call in a constructor - I knew the shit's gonna go down in the comments 🤣
@Argoon1981
@Argoon1981 7 ай бұрын
I have seen professional famous code bases, made by programmers with decades of experience, putting function calls on constructors. Some people in the programming circle, is to hellbent on rules and "good practices" and never stop to think, if they really matter and if they really help on making good fast software.
@kostiapereguda
@kostiapereguda 7 ай бұрын
Actually, there is nothing inherently wrong in calling instance methods from inside constructors. If it was something illegal, than the compiler would not allow such code to be compiled. The actual constructor is called on the object which has already been fully allocated and it’s purpose it to bring it to the valid state for the user to use, arbitrary code can be executed from with it. The only reason why people say “it’s bad to call instance methods from within a constructor” is because the arbitrary instance method generally relies on the object to be in a fully initialized/valid state, which is not the case if you call such a method before you have finished setting up the object’s state in the constructor. But other than that it is perfectly valid to execute arbitrary code inside a constructor. The problem here I guess is that people don’t like the fact that the game is launched on the creation of a game object, because at the moment it is constructed it is immediately played, and then the actual object serves no purpose to the user, except that he has to call a Clean() method on it, which might as well be called by the constructor too. But at this point you no longer have an object, you just have a function😅
@nevemlaci2486
@nevemlaci2486 7 ай бұрын
​@@kostiapereguda The compiler allows you to create a vector of references too, while it is ill formed
@kostiapereguda
@kostiapereguda 7 ай бұрын
@@nevemlaci2486 first of all, no, it doesn’t. And secondly, this doesn’t invalidate my point about constructors
@nevemlaci2486
@nevemlaci2486 7 ай бұрын
@@kostiapereguda It doesn't let you create a classic array of them, but I'm pretty sure it did allow me to create an std::vector out of references
@kostiapereguda
@kostiapereguda 7 ай бұрын
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
@reup6943
@reup6943 7 ай бұрын
That's because startGame() is executing the game loop and not just initializing stuff as the name should indicate. With proper naming like initGame() and additional runLoop() I don't think you would mind having initGame() in your constructor and then call runLoop() after the object is created. That point was made afterwards.
@kostiapereguda
@kostiapereguda 7 ай бұрын
@@reup6943 the point that was made later in the video was quite different from what I said. The author’s point was that you don’t even need an object to start up and run game, you can use a function (which is essentially the same thing as I said at the beginning of my comment, so I agree with this). But my actual point wasn’t about this, my actual point was that if you are going to represent a game as an object, then you should also give the user a proper control of that object. Perhaps you want to launch the game from a different thread, or you want to pass the representation of that game around, store it somewhere without actually launching the run loop, perhaps you have a launcher that lets you pick from a set of games, or perhaps you have a load/save functionality that constructs and returns you an instance of the game object on load, whatever. The reason I wrote this comment afterall, is because I’ve indeed seen real people writing code that launches a game loop in the constructor of a game object. Sure, it may be cool for a code like “new Game();” to actually launch the game, but i personally don’t think that this is the right way to use objects
@dm121984
@dm121984 6 ай бұрын
Also, moving the object to the stack as he does in an intermediate state is not quite the same as the original code - the execution of the destructor/clean function is moved after the "pause" command, which means if your engine is reporting warnings or errors in the shutdown state it may not be visible to the user (assuming they execute the game from a gui rather than a command terminal). I like how he ends up with the static function RunGame() but his intermediate code is not quite the same as a refactor.
@Omsip123
@Omsip123 7 ай бұрын
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
@antagonista8122
@antagonista8122 7 ай бұрын
Not really, exception invokes stack unwinding so if you don't explicitely mess with catching it, there is no possibility to access partially constructed object, but even then, catch scope is different scope than try scope.
@Anonymous_0range
@Anonymous_0range 7 ай бұрын
Also it makes creating subclasses a headache. I feel like the ShooterGame class probably shouldn’t exist though so whatever.
@kenarnarayaka
@kenarnarayaka 7 ай бұрын
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
@gustavbw 7 ай бұрын
"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.
@skeleton_craftGaming
@skeleton_craftGaming 7 ай бұрын
Enums are objects in Java...
@hamzaiqbal7178
@hamzaiqbal7178 6 ай бұрын
How the f did you put a function in an enum. It may be clear that I've used java the least
@gustavbw
@gustavbw 6 ай бұрын
@@hamzaiqbal7178 The dark side is a pathway to many abilities some would consider... unnatural. public enum Something { Value(() -> System.out.println("hi")); public final Runnable func; Something(Runnable func){ this.func = runnable; } }
@IStMl
@IStMl 6 ай бұрын
You're a danger to society
@fitmotheyap
@fitmotheyap 5 ай бұрын
I understand the others, but enum what?
@jonnyrichards
@jonnyrichards 6 ай бұрын
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.
@starup4960
@starup4960 7 ай бұрын
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.
@bart2019
@bart2019 7 ай бұрын
As a non C++ programmer myself, I wouldn't even expect an object there, but just an empty typed variable. So, no, I would definitely not expect this declaration to run anything.
@Argoon1981
@Argoon1981 7 ай бұрын
But that is not a simple variable declaration, is a class object creation on the stack (does automatic cleanup) and a c++ programmer should know that, like he/her should also know that creating a class object, is calling the default constructor or other custom constructor that may have been defined, so as C++ programmers, they should have in mind that depending on who wrote the class, some functions could very well be called/run at object creation time. Is very common despite what some here claim to be "bad practice", to put function calls in a class constructor. Also he never said that was what people should do, he only showed that is something you can do, very different things. And IMO a good way to teach what is happening behind the curtain when creation a c++ class object.
@nikkiofthevalley
@nikkiofthevalley Ай бұрын
​@@Argoon1981It isn't, no, but standards exist for a reason. Constructors should construct, not execute.
@angela_jx
@angela_jx 5 ай бұрын
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.
@GreenFlame16
@GreenFlame16 7 ай бұрын
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 :)
@birdegop
@birdegop 7 ай бұрын
I enjoyed this video much more than previous ones. This one is just straight forward, precise, no out of context talking and opinions.
@mysteriousdbs2534
@mysteriousdbs2534 7 ай бұрын
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.
@sergeikulenkov
@sergeikulenkov 7 ай бұрын
Yeah, it's really interesting to hear your thoughts on creating a pointer in a function call. Please make the next part
@not_ever
@not_ever 7 ай бұрын
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.
@samuelazeredoo
@samuelazeredoo 6 ай бұрын
Please continue reviewing this code! This type of video is very helpful for beginners
@SuperiorZeeko
@SuperiorZeeko 7 ай бұрын
Your code reviews are really fun to watch
@SkrekkLich
@SkrekkLich 5 ай бұрын
I like the approach to look at your code like you are shipping a library other devs are using. It often makes sense to just use pseudo code or stubs to get a feel for what is needed. PdfTool pdf; pdf.Load("path"); auto page = pdf.Pages.Add("P1"); page.AddText("hello"); pdf.Pages["P1"].AddText(" world"); page.Elements.RemoveAllByContent("hello"); and so on ... just from this little example, you can see, what classes you need and what the code flow for the user will be etc...
@AppleHair
@AppleHair 7 ай бұрын
I love seeing classes like these get crammed into a single function. It's just so satisfying!
@TheAxeForgetsTheTreeRemembers
@TheAxeForgetsTheTreeRemembers 7 ай бұрын
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.
@jakobstengard3672
@jakobstengard3672 4 ай бұрын
So refreshing to listen to you and basically agree with everything you said. That is usually not the case with me!
@GoncaloFerreira
@GoncaloFerreira 6 ай бұрын
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. :)
@halalos
@halalos 6 ай бұрын
i've been following your videos since i started learning programming since then im a medior dev, and its so nice to see that in the beginning i was learning something new every video, now i was thinking almost the same stuff, and would refactor it in a very similar way :D you helped me improve a lot thanks for your work
@PBlague
@PBlague 7 ай бұрын
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!
@betterfly7398
@betterfly7398 7 ай бұрын
That "I don't wanna talk about it" at the end sounded just right lol
@hansdietrich83
@hansdietrich83 7 ай бұрын
The constructor starting the game is not good very self explanatory
@Entropy67
@Entropy67 7 ай бұрын
For a game object like this generally I would keep that start game function. But I think the point he was trying to make was that since there is basically nothing in the class you might as well combine them. Honestly I wouldn't take an object oriented approach...
@OkOkOkIMightKnowYou
@OkOkOkIMightKnowYou 7 ай бұрын
I really like static create functions where you do heap allocation for the object and then a destroy method that cleans up and deletes
@magyx3014
@magyx3014 7 ай бұрын
Would love to see a continuation on this!
@NightFyreTV
@NightFyreTV 6 ай бұрын
had absolutely no idea you could rename variables and have it update across an entire source. Ive been wanting to update some variable names but had no mind to dig through every source file and update the variable name. Wow, thanks cherno
@Stdvwr
@Stdvwr 5 ай бұрын
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
@michaelkjellander9370
@michaelkjellander9370 4 ай бұрын
Yep, it's a typical horrible code review with bad advice for beginners. I'm not a C++ dev, but I'm a senior dev in object oriented languages, and the original code looked perfectly fine to me. This guy should stick to giving feedback on code that's objectively bad. It's frustrating getting your code reviewed by interviewers since their personal preferences always differs, which makes it impossible to satisfy them.
@LunaticEdit
@LunaticEdit 6 ай бұрын
The second I saw std::vector my heart skipped a beat, kinda like a car pulls out in front of me on the highway or something.
@BurninVinyl
@BurninVinyl 6 ай бұрын
I'm very new to C++ but the first thing that I didn't understand was the destructor on the first line of the class. Anyway thanks Cherno for the video, every time it's like a back to basics for me.
@Noriyak1
@Noriyak1 7 ай бұрын
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
@gfabasic32
@gfabasic32 7 ай бұрын
I am learning lots. Thanks.
@CowboyCoder21
@CowboyCoder21 7 ай бұрын
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.
@K3rhos
@K3rhos 5 ай бұрын
10:50 Actually that was what I was doing for years in C++, but with time, by also using other languages, I got obsessed by adding classes/structs for everything, even in C++, Idk I just think it's cleaner and more organized. But I agree, in the case showed in this video, it make sense not using a class. One thing I found weird particularly about this code though is how this main is structured. I mean in pratically every cases when creating a game (or every app in general) you will want to do something like this: int main() { Game game; while (game.IsRunning()) { game.Update(); } game.Stop(); } I mean the way this main is done is weird to me, you generally want to declare an object "Game" or "Application", check if it's running, update loop, and then call a Stop() fonction (or just add everything in the destructor of the class, but I usually prefer to have a "Close" or "Stop" function, it make things more clear and cleaner.
@GreenFlame16
@GreenFlame16 7 ай бұрын
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
@oglothenerd
@oglothenerd 7 ай бұрын
These videos are very useful for learning C++.
@Manimanocas
@Manimanocas 7 ай бұрын
I might still not know anything but this video really useful so I can organize better, thank you
@AeroSW
@AeroSW 4 ай бұрын
I think the reason people use heap allocation everywhere with "new" and "delete" rather than relying on the stack is how C++ is taught in college. Until I took on learning more about C++ myself during university, I use to do this as well. Utilizing "new" and "delete" everywhere seems to be something the professors learnt from Java for some reason.
@NeverEngineDev
@NeverEngineDev 6 ай бұрын
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.
@BurninVinyl
@BurninVinyl 6 ай бұрын
should then be an abstract class for that?
@anon_y_mousse
@anon_y_mousse 7 ай бұрын
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.
@distortions
@distortions 7 ай бұрын
i love these style of videos, please do more :))
@harywhiteproductions
@harywhiteproductions 7 ай бұрын
Would love to see a video about pointers as you suggested at the end.
@MrAbrazildo
@MrAbrazildo 7 ай бұрын
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.
@matu.ayrton
@matu.ayrton 6 ай бұрын
I learnt a lot! thanks for uploading
@aikslf
@aikslf 7 ай бұрын
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()
@benjaminlehmann
@benjaminlehmann 5 ай бұрын
Yes please to the video on your feelings about pointers
@SnakeEngine
@SnakeEngine 7 ай бұрын
All good, but I would not stack allocate Engine. A non trivial engine might be bigger in memory than the tiny stack.
@ade5324
@ade5324 7 ай бұрын
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
@leviathanx0815
@leviathanx0815 5 ай бұрын
If you have a software with multiple threads and those threads eventually depend on each other, just exiting your program might end up in a mess. It might look to you as your software exists "normally", but in reality, especially with aforementioned dependencies, you can cause "internal crashes", to phrase it in simple terms. So, for example by popularity, Windows attempts to shutdown your threads, they hang, the shutdown hangs, eventually up to 30 seconds later, the software is closed. Internally it (the system) would call some exception handling to get things untangled and hopefully freed correctly. For you, depending on your compiler and compiler settings (release build and debug build can have sometimes really big differences in this type of behavior), you might see absolutely nothing, besides the 30 seconds of silence, or you see a crash report. I know a lot of people who shrugged it off with "Software is closed, why should I care?" I mean, imagine your threads, which might still be up, still work(ed) on resources, which are not properly closed or they might lose access to instances, in the middle of their routine, because the instance's destructor was called "randomly". That's bound to create data loss or corruption at some unlucky point. You surely don't want that to happen.
@ade5324
@ade5324 5 ай бұрын
@@leviathanx0815 yea, i don't like multi threating, its hard and i don't understand it
@Vicente_Lopes_Senger
@Vicente_Lopes_Senger 7 ай бұрын
Annoying when you spend 5 minutes writing a comment and the KZbin bot deletes it because of technical jargon, I'm assuming.
@superscatboy
@superscatboy 7 ай бұрын
Could be because it saw a lot of unusual symbols and made a "better safe than sorry" assumption to try and avoid an unknown attack of some kind. Maybe. Some stuff that YT auto-deletes is just insane so it could just as easily be that someone at Google is offended by the word "template" or something lol
@siniarskimar
@siniarskimar 7 ай бұрын
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.)
@rowdyriemer
@rowdyriemer 7 ай бұрын
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().
@atijohn8135
@atijohn8135 6 ай бұрын
7:00 stack allocations aren't always better: if you're allocating big objects, it's often better to have it placed outside of the stack e.g. using the static keyword than to allocate them on the stack directly, since it can lead to a stack overflow if your function call graph goes deep enough and you have a lot of big objects on the stack. in this case it's not that bad, since it's just 40 bytes, but a 4kB massive blob (and a GameEngine object could as well weigh that much) would be better put in static storage
@amber1862
@amber1862 7 ай бұрын
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.
@ditz3nfitness
@ditz3nfitness 6 ай бұрын
Love this!
@sanderbos4243
@sanderbos4243 7 ай бұрын
Great review
@sfperalta
@sfperalta 7 ай бұрын
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.
@cprn.
@cprn. 7 ай бұрын
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. 😆
@mr.mirror1213
@mr.mirror1213 7 ай бұрын
Ocd hitting cherno hard😂🤣 (pls bring back ray tracing series)
@TheAxeForgetsTheTreeRemembers
@TheAxeForgetsTheTreeRemembers 7 ай бұрын
This is not ocd, this is senior developer reviewing junior code. Cherno humbly said all of this was his opinion, but I think it's more good practice coming from experience, than opinion.
@malekith6522
@malekith6522 7 ай бұрын
@@TheAxeForgetsTheTreeRemembersActually those stuff is expected to be known from Junior engineer. This code is more of high schooler.
@FirstnameLastname-jd4uq
@FirstnameLastname-jd4uq 7 ай бұрын
@@TheAxeForgetsTheTreeRemembersbesides ocd isn't just being "neat", it's SO much more than that, it's a genuine mental illness with many different ways it can present.
@rowdyriemer
@rowdyriemer 7 ай бұрын
I wouldn't start in the constructor, but I'm not dogmatically against it.
@schlimmpf158
@schlimmpf158 6 ай бұрын
there is at least one usecase for cleaning up memory when exiting the program: if you're using address sanitizer it wont give you a bunch of false positives
@anfay27
@anfay27 7 ай бұрын
15:59 that's good idea!
@adampecenka1909
@adampecenka1909 6 ай бұрын
this makes me feel like i need to learn more keyboard shortcuts, even tho i know bunch, but i aint that fast
@sebastianschweigert7117
@sebastianschweigert7117 6 ай бұрын
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?
@mertcanzafer9901
@mertcanzafer9901 7 ай бұрын
I want more code review series videos
@BartoszWajsowicz
@BartoszWajsowicz 26 күн бұрын
my homie cherno spittin' straight facts here no capp
@iamKuff
@iamKuff 6 ай бұрын
Very helpfull thankyou!!
@Argoon1981
@Argoon1981 7 ай бұрын
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.
@ning_nang
@ning_nang 7 ай бұрын
More of these pls
@justinkendall5647
@justinkendall5647 7 ай бұрын
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.
@TacoGuy
@TacoGuy 7 ай бұрын
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
@TheRealZitroX
@TheRealZitroX 4 ай бұрын
7:40 love your code, but auto should be used when it’s clear what type the variable gets 🙌
@joshdoucette6802
@joshdoucette6802 3 ай бұрын
A lot of knowledge right here
@sandipdas7206
@sandipdas7206 7 ай бұрын
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"
@baileyharrison1030
@baileyharrison1030 5 ай бұрын
If you're using Visual Studio and you don't want the console to immediately close, don't do anything in your code, just change Tools -> Options -> Debugging -> Automatically close the console window when debugging stops. EZ
@lenonkitchens7727
@lenonkitchens7727 6 ай бұрын
I agree with moving ShooterGame::Clean() to the destructor, but I strongly disagree with moving ShooterGame::StartGame() to the constructor. The reason being that, I assume, StartGame() can fail, which would mean the constructor can fail, and constructors should never contain conditions that fail in a non-recoverable way, if avoidable. Granted, in this case it's not a big deal because it's the main class of the program, and any failure just results in an exit condition anyway, but in larger projects where a class failing to do it's job is not necessarily fatal, it could be an issue. Having failure conditions in constructors is a bad habit to get into. That's just my two cents.
@feschber
@feschber 7 ай бұрын
As a linux user I was today years old when i learned that windows has a distinction between "console" and "windows" apps lmao 🤡
@mishaerementchouk
@mishaerementchouk 5 ай бұрын
How do you create GUI applications on Linux? Do you write, say, all Qt’s boilerplate every time from the scratch?
@XENON2028
@XENON2028 5 ай бұрын
that's not a bad thing though
@konstantinsotov6251
@konstantinsotov6251 4 ай бұрын
equivalent to windows API would be Xorg or Wayland API - these are "providers" for linux user interface. Though they are not interchangable. So alternatively, you use FLTK, Raylib (initially made for games, but can be used for UI), GTK and Qt, there is a variety of optionz
@konstantinsotov6251
@konstantinsotov6251 4 ай бұрын
I can't edit a comment for some reason, but i'll add that fltk, gtk and raylib can also be used on windows, mac e.t.c..
@mishaerementchouk
@mishaerementchouk 4 ай бұрын
@@konstantinsotov6251 right, and if one uses QT Creator or GNOME Builder, then, for a new project, one needs to choose between “window” or “console” application. For the latter, a lot of unnecessary boilerplate will not be generated.
@radoslavdimitrov7505
@radoslavdimitrov7505 7 ай бұрын
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
@jawadsrour8490
@jawadsrour8490 7 ай бұрын
great one!
@A_Wirla
@A_Wirla 7 ай бұрын
That was useful, thanks
@WhoLeb7
@WhoLeb7 7 ай бұрын
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
@user-sl6gn1ss8p
@user-sl6gn1ss8p 7 ай бұрын
Cherno Code Review Cheat Sheet: - See "new"; - Rewrite file; - Don't look at anything else.
@mahee96
@mahee96 7 ай бұрын
Yeah half of what all is said is almost just personal preference…was the earlier code ugly…not very much except missing some constructor, destructors, and having a clearcut API segregation…
@Greenstack
@Greenstack 7 ай бұрын
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.
@poleve5409
@poleve5409 Ай бұрын
starting game in constructor is crazy
@Petroupas
@Petroupas 7 ай бұрын
I could watch refactoring for hours
@Netryon
@Netryon 7 ай бұрын
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.
@absorbingdude
@absorbingdude 7 ай бұрын
Thank you for today's stream, I enjoyed CLion all the way)
@Asdayasman
@Asdayasman 7 ай бұрын
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.
@0LoneTech
@0LoneTech 3 ай бұрын
Remarkable how that four letter initialism doesn't relate to cleanup time.
@b4ttlemast0r
@b4ttlemast0r 7 ай бұрын
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.
@yaaashua
@yaaashua 4 ай бұрын
Hi! I recently started to watch your "Code Review" playlist and I love it. I am learning a lot, even if I am a programmer with some years of experience. but.... I got one question: in many projects, the "application" variable ( in this video the ShotterGame class ) is initialized as a pointer, a heap allocation object: - - - ShooterGame* game = new ShooterGame(); and your advice is always to make it a stack allocated object... like - - - ShooterGame game; Then... when you show the code for your Hazel game... you are also doing the same - - - Application* app = CreateApplication(argc, argv); is there a reason for this?
@y4ni608
@y4ni608 4 ай бұрын
Probably because this game is a small game we are talking about some bytes his Application class is probably more mb's
@Anthonincolas
@Anthonincolas Ай бұрын
the tild before a cosntructor means that gonna be the destructor ?
Ай бұрын
Brilliant. Less OO cargo culting :)
@marco_foco
@marco_foco 5 ай бұрын
I agree with most of the comments, including the changed behavior of moving the destruction point with RAII) but I believe the (intermediate) advice to move startGame inside the constructor is bad advice, especially if the class is used for something else, and is used for inheritance and might contain (in future) virtual functions that are overridden by other functions. The risk here is that the virtual functions became counterintuitive because "startGame" is called before the class is completely built, and the object might not be fully initialized (e.g. a subclass won't be). If you want to prevent that, the class should be marked final (but yeah, you removed it).
@noscopedquicksoper6234
@noscopedquicksoper6234 7 ай бұрын
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
@anon1963
@anon1963 7 ай бұрын
did he mention certain platforms? I thought he specifically talked about operating systems?
@maltebp
@maltebp 7 ай бұрын
Is it really a problem in that case? If you don't have an operating system, then I suppose there is no one who manages ownership except what you do within your progrram, and so you cannot "free" your program's memory anyway, because no one else can claim it. If we're talking about cleaning up the memory, my understanding is that freeing memory does nothing to clean it - i.e. the memory bits are not set to 0 or anything like that. Except, I suppose, if you do that in your own delete operator implementation.
@marcotroster8247
@marcotroster8247 6 ай бұрын
Usually a microcontroller only runs a single app. So yeah, this device only runs the damn game. When you quit, the machine has nothing left to do. So freeing memory shouldn't matter.
@adam3141
@adam3141 5 ай бұрын
Usually with a microcontroller, you do not ever want to hit the end of the main function and if you do, then you need to go back to your design and rewrite it. What you find is that after the return of main, the destructor code is run and then it hangs indefinitely. It is possible, and I have seen it on STM32 micros that interrupts are not disabled so that if you don't clean up properly, your ISR could still trigger and cause problems. However, speaking more to operating system applications, I would always, again ALWAYS try to clean up before you exit your application. 1. Your resources might need to be released in a specific way, closing a file might satisfy the operating system but might be problematic for your specific circumstance. 2. Some resources that are shared, i.e. you might have exclusivity while your program runs and if you change this, you better change this back to how you were given it because the OS isn't going to do this for you. Example is the TTY state in Linux. If I change the baud rate of a serial port from 9600 to 115200 in my program, I need to set this back to 9600 when I terminate because there may be other software that relies on this 9600 rate. 3. It is just good housekeeping. That is, don't rely on others to clean up your mess.
@0LoneTech
@0LoneTech 3 ай бұрын
He specifically talked about it being a console game, where that cleanup is typically important to do things like restore character echo, turn raw input off, and switch to text instead of graphic characters.
@TotalImmort7l
@TotalImmort7l 6 ай бұрын
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.
@sammyfromsydney
@sammyfromsydney 5 ай бұрын
None of this is so untidy that it matters either way, especially so close to the entry point where you're not going to end up with a memory leak.. Perhaps the change from StartGame to RunGame might add clarity. Changing things for preference is a bad idea. You can get away with it on smaller projects.
@Cruxics
@Cruxics 7 ай бұрын
I hope some of my Jr engineers give this a look.
@stiliok
@stiliok 7 ай бұрын
Huh?
@Cruxics
@Cruxics 7 ай бұрын
@@stiliok let those gears turn buddy.
TERMINAL GAME ENGINE! // Code Review
23:59
The Cherno
Рет қаралды 75 М.
Learn RAG, LangChain, Vector DB's, with this project, FULL COURSE
1:29:21
PropTech Founder
Рет қаралды 1,7 М.
ТВОИ РОДИТЕЛИ И ЧЕЛОВЕК ПАУК 😂#shorts
00:59
BATEK_OFFICIAL
Рет қаралды 5 МЛН
ТЮРЕМЩИК В БОКСЕ! #shorts
00:58
HARD_MMA
Рет қаралды 2,5 МЛН
كم بصير عمركم عام ٢٠٢٥😍 #shorts #hasanandnour
00:27
hasan and nour shorts
Рет қаралды 4,7 МЛН
FASTEST Way To Learn Coding and ACTUALLY Get A Job
10:44
Brian Cache
Рет қаралды 1,2 МЛН
Is this the WORST CODE I've EVER SEEN? // Code Review
24:28
The Cherno
Рет қаралды 73 М.
He wrote this out BY HAND? // Code Review
24:01
The Cherno
Рет қаралды 176 М.
Stack vs Heap Memory in C++
19:31
The Cherno
Рет қаралды 579 М.
From Scala to Silicon: The Chisel Journey with Jack Koenig
1:34:34
Scala for Fun & Profit
Рет қаралды 814
BETTER Header Files and Preprocessor Debugging
24:26
The Cherno
Рет қаралды 74 М.
Stop using std::vector wrong
23:14
The Cherno
Рет қаралды 148 М.
Naming Things in Code
7:25
CodeAesthetic
Рет қаралды 2,2 МЛН
I made it FASTER // Code Review
38:46
The Cherno
Рет қаралды 549 М.
WHY did this C++ code FAIL?
38:10
The Cherno
Рет қаралды 293 М.
ТВОИ РОДИТЕЛИ И ЧЕЛОВЕК ПАУК 😂#shorts
00:59
BATEK_OFFICIAL
Рет қаралды 5 МЛН