Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Feedback for my TicTacToe game.  (Read 3407 times)

0 Members and 1 Guest are viewing this topic.

sofnir

  • Newbie
  • *
  • Posts: 3
    • View Profile
    • Email
Feedback for my TicTacToe game.
« 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 :)




« Last Edit: June 13, 2017, 02:14:48 pm by sofnir »

Elias Daler

  • Hero Member
  • *****
  • Posts: 599
    • View Profile
    • Blog
    • Email
Re: Feedback for my TicTacToe game.
« Reply #1 on: June 13, 2017, 01:06:09 pm »
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.
Tomb Painter, Re:creation dev (abandoned, doing other things) | edw.is | @EliasDaler

sofnir

  • Newbie
  • *
  • Posts: 3
    • View Profile
    • Email
Re: Feedback for my TicTacToe game.
« Reply #2 on: June 13, 2017, 02:12:04 pm »
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 :) 

Elias Daler

  • Hero Member
  • *****
  • Posts: 599
    • View Profile
    • Blog
    • Email
Re: Feedback for my TicTacToe game.
« Reply #3 on: June 13, 2017, 02:13:56 pm »
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. :)
Tomb Painter, Re:creation dev (abandoned, doing other things) | edw.is | @EliasDaler

 

anything