Jump to content
  • Advertisement
MaizeDaniele

[CODE REVIEW REQUEST] Pong clone with C++ and SFML

Recommended Posts

Hello,
I'm following this article suggestions in order to improve my basic gamedev knowledge. 

I've completed the first game in the list: Pong, using C++ and SFML. The thread suggests to ask for code review after finishing a project, so this is the game repo: 
https://github.com/MaizeDaniele/Pong

 

Let me know what I should have done differently.

Thanks in advance,
MaizeDaniele
 

Edited by MaizeDaniele

Share this post


Link to post
Share on other sites
Advertisement
1 hour ago, MaizeDaniele said:

my basic gamedev knowledge...

completed the first game...


Based on the code, you either have some experience as a programmer or you followed a tutorial. Which one?

 

Edited by fleabay

Share this post


Link to post
Share on other sites
25 minutes ago, fleabay said:


Based on the code, you either have some experience as a programmer or you followed a tutorial. Which one?

 

I've used Unity for a while, developed some basic 2D mobile games. 
I'm studying some patterns from:
http://gameprogrammingpatterns.com/
for example I tried to use the command pattern for input, and the game event pattern. 

I wanted to improve my C++ knowledge. I read a basic book on C++ and some introductory tutorial on the language (ex TheChernoProject C++), but I didn't followed a tutorial specific on pong with sfml. 

By "first game" I meant the first on the list.

Edited by MaizeDaniele

Share this post


Link to post
Share on other sites
1 hour ago, MaizeDaniele said:

By "first game" I meant the first on the list.

I didn't mean to sound like I was calling you out. :D

Share this post


Link to post
Share on other sites
8 minutes ago, fleabay said:

I didn't mean to sound like I was calling you out. :D

Oh sorry, I didn't want you to think that 😅
I just wanted some suggestions about the c++ project, because I've never built a game without an engine (ex. Unity) before.  

Share this post


Link to post
Share on other sites
3 hours ago, MaizeDaniele said:

The thread suggests to ask for code review after finishing a project, so this is the game repo:

This is all just low-hanging fruit based on a quick look at some of the code (some of the things pointed out here come up pretty commonly in code review requests):

- In C++, the use of the preprocessor for constants is generally discouraged for various reasons (at or towards the top of that list would probably be that preprocessor macros are scope-unaware). C++ provides (arguably) better tools for this, such as 'const' and 'constexpr'.

- Very minor, but I'd suggest expressing the 'degree to radians' constant in terms of pi (e.g. 'pi / 180.0') rather than expressing it as a literal. Since it's dependent on a constant you've already defined, there may be no particular reason to express it directly (which, among other things, runs the risk of getting it wrong). Technically you could probably also get pi from a trig function, although that's probably less important. (Although the chances of getting these constants wrong is obviously low, the general goal here is to minimize the number of hard-coded values - especially values that have invariant relationships to one another - and therefore reduce the potential for error.)

- It's idiomatic to pass instances of non-trivial types like std::string by reference (constant reference if mutability isn't required, which I would think is usually the case), which may avoid some overhead.

- How much to use 'const' is a matter of taste and opinion, but if you want (this is my preference), there are some things you can or might be able to make constant that aren't currently (e.g. immutable function arguments, member fields, locals, etc.).

- In the 'Game' class, you're creating and deleting m_GameSceneManager manually, but it would probably be safer and more idiomatic to use e.g. std::unique_ptr for this. Among other things, this would remove the need for deleting it explicitly in the destructor and elsewhere.

- '#pragma once' is widely supported, but technically non-standard. Just something to be aware of, depending on what your portability goals are.

- Here:

if (m_GameSceneManager == nullptr)
    m_GameSceneManager = ...;

You have a control structure without braces. Opinions may differ on this, but it's often recommended to always include the braces (one reason being that if you decide to add additional statements later, you won't run the risk of introducing bugs by forgetting to add the braces, and also won't have to go to the trouble of adding them after the fact at all).

- Just as a point of interest, 'return 0' in main() is the default behavior, so its inclusion is optional. I suppose some might recommend including it for clarity regardless (as you're doing).

There are probably some other things, but I didn't look at all the code, and in any case that's probably enough for one post. Overall it looks very good :)

Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!