SFML community forums
General => SFML projects => Topic started by: sofnir on June 11, 2017, 02:45:12 pm
-
Hi everybody
I am new here and i want to show you my project. It is TicTacToe game, this game is very simply but i tried focus on code. Link to my github: https://github.com/sofnir/TicTacToe If you can please look at my code and give me some tips how to programming better, tell me about my mistakes and what can i do in the better way.
Thanks for all and have a nice day :)
(https://user-images.githubusercontent.com/17216605/27080564-bc93149e-503c-11e7-96d6-b24a699b9e22.png)
(https://user-images.githubusercontent.com/17216605/27080565-c17ef2b6-503c-11e7-80b4-ce2920eb350b.png)
-
It looks pretty good, great job!
Some things I want to point out:
* Always use curly braces with for/if even if there's only one statement inside because if you add another statement and forget to add curly braces, this bug will be not so easy to trace.
* Don't create empty destructors, they're automatically generated for you.
* Use enum class instead of enum, because enum class is more type-safe and doesn't "leak" in outer scope.
* Try not to use raw pointers, use unique_ptr instead (especially for GameState stack). This makes memory management easier and safer.
* There's no need to use curly braces in switch/case statements for cases, they're only required if you create some object inside some case. But in other cases "break" is all what is needed.
* Game.h includes <iostream> which is not needed there. Maybe it was used for debugging and you forgot to remove it, but it's a pretty big header, even if you need to include it (e.g. you're passing some std::ostream as parameter), better include <iosfwd> header.
* Game.h also includes <SFML/Graphics.hpp> which is just a header which includes all the graphics related headers and may blow up your compilation time a lot. Include stuff you need directly (in this case you only need to include <SFML/Graphics/RenderWindow.hpp>).
* If you're overriding some method, there's no need to put virtual in the function declaration.
That's just some small things I noted, but again, I like the code overall, it's clean and I like how much you separated it instead of putting it all in one big messy file.
-
It looks pretty good, great job!
Some things I want to point out:
* Always use curly braces with for/if even if there's only one statement inside because if you add another statement and forget to add curly braces, this bug will be not so easy to trace.
* Don't create empty destructors, they're automatically generated for you.
* Use enum class instead of enum, because enum class is more type-safe and doesn't "leak" in outer scope.
* Try not to use raw pointers, use unique_ptr instead (especially for GameState stack). This makes memory management easier and safer.
* There's no need to use curly braces in switch/case statements for cases, they're only required if you create some object inside some case. But in other cases "break" is all what is needed.
* Game.h includes <iostream> which is not needed there. Maybe it was used for debugging and you forgot to remove it, but it's a pretty big header, even if you need to include it (e.g. you're passing some std::ostream as parameter), better include <iosfwd> header.
* Game.h also includes <SFML/Graphics.hpp> which is just a header which includes all the graphics related headers and may blow up your compilation time a lot. Include stuff you need directly (in this case you only need to include <SFML/Graphics/RenderWindow.hpp>).
* If you're overriding some method, there's no need to put virtual in the function declaration.
That's just some small things I noted, but again, I like the code overall, it's clean and I like how much you separated it instead of putting it all in one big messy file.
Thanks a lot! I am very happy that you spent your valuable time and wrote my code. I understand your all tips and i will upgrade my code :)
-
No problem! I recommend reading Code Complete, Effective C++, Effective Modern C++ and Clean Code if you haven't already. They're great for improving subtle things like that. :)