Jump to content
  • Advertisement
reders

Yet Another Pong

Recommended Posts

Hi, everyone!

I "finished" building my first game. Obviously Pong.
It's written in C++ on Visual Studio with SFML.

Pong.cpp

What do you think? What should I consider doing to improve the code?

Thank you very much.

 

EDIT: added some screenshot and .zip file of the playable game

 

Pong.zip

Screenshot (1).png

Screenshot (4).png

Edited by reders

Share this post


Link to post
Share on other sites
Advertisement

classes are used to associate variables and functions with a common name. I notice you have a lot of variables that can indeed be grouped under a common name, this would be object-orientation. An obvious association to me are your actual game objects: ball, paddle. Right now you have graphics objects and do work on them all over the place, you could instead put these graphic objects into a class and add methods to do the related work. Another observation is that you have nameless constants all over, you could actually make these constants or defines to give them a name. 

Share this post


Link to post
Share on other sites

Thank you very much. I'll work on this. One more question. What's the best way to divide the code in different files like headers and such? Is there a standard?

Share this post


Link to post
Share on other sites

Pong is an extremely simple game, and it is easy to over-engineer.

I'd ignore the comments about classes for Pong.  The code you've got there does not need them.

The only variable really are the position and velocity of the ball, the position of the paddles, and the scores.  You can add more fluff, but that's all the game requires.  That does not lend itself to game objects with classes and interfaces and interdependencies.  I've put together a pong game for the Challenge this month and it was about 90 lines before I started adding the fluff. Pong does not need to be complex since typically the complex stuff of graphics and sound are handled by libraries.

That is partially why Pong it an excellent beginner exercise.  Larger projects certainly need it, but this doesn't.

 

For a code review...

Lines 15-47, you don't need to make all of those static.  Global variables are static by definition.  

I thought it was interesting that you made some global constants, but you also scattered numbers around the code.  You should probably put all your constants up there, but more on that below.

Overall you place multiple statements on a single line, that is not a typical pattern.  For example, lines 69-75 contain 32 statements.  Why did you do that?  Typical C++ code has one statement per line, even one declaration per line in most cases.  Trying to cram 32 statements into 6 lines makes the code difficult to read.

If each is a block of functionality, consider adding a comment to each block, or pulling it out to a function.

From your code example, perhaps instead of three lines, try this:

//Old code, this seems difficult for me to read
	ball.setRadius(10);	ball.setOrigin(10, 10);	ball.setFillColor(ball_color);	ball.setPosition(640, 360);
	paddle1.setFillColor(paddle1_color); paddle1.setOrigin(8, 60); paddle1.setPosition(35, 360);
	paddle2.setFillColor(paddle2_color); paddle2.setOrigin(8, 60); paddle2.setPosition(1245, 360);

// Suggested version

    // Initialize ball and paddles
	ball.setRadius(10);     //TODO: Replace all these numbers with named constants
	ball.setOrigin(10, 10);	
	ball.setFillColor(ball_color);	
	ball.setPosition(640, 360);

	paddle1.setFillColor(paddle1_color); 
	paddle1.setOrigin(8, 60); 
	paddle1.setPosition(35, 360);

	paddle2.setFillColor(paddle2_color); 
	paddle2.setOrigin(8, 60); 
	paddle2.setPosition(1245, 360);

In that snippit of code you've got a lot of magic numbers.  These are difficult to maintain.  It is usually easier to provide named constants that are all found in one convenient location. Then you can change one constant and it is reflected everywhere in the code.  As written you may decide you want to change the paddle origin value from 60 to 48, and so you'll be searching the code for everywhere that has a 60 in it. Then when you find a 60, you'll need to make sure it is the right 60 and not used for something else. And even then, maybe for some reason you didn't use 60 but were using another value to represent something, or maybe you already added to values together but now you want to modify one of them.  Lots of reasons, always use named constants. They should be added up near your other constants used for colors up around line 20.

 

As for dividing code between files, typically the approach is to block them together by functionality, which usually means by feature or by class. Sometimes a third file will be used when using something like the pImpl paradigm, where the publicly used interface doesn't need to contain all the inner details which can help reduce build times on large projects.

For organization, it depends. Often (but not always) directory hierarchies follow namespace hierarchies, and often (but not always) large subsystems and external subsystems go in their own hierarchies. The exact details vary from project to project depending on what works for the team.

 

 

Overall it looks good for this skill level, and looks like it does the job of being a good pong clone.

Share this post


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

Pong is an extremely simple game, and it is easy to over-engineer.

I'd ignore the comments about classes for Pong.  The code you've got there does not need them

I disagree, how is adding some classes over-engineering? In this particular case he did not add classes because he does not understand how to use them. There's a lot of code in there that could be a lot more readable and correct with objects, why favor less for the sake of "engineering less"? If there is one thing this fellow needs to learn about, judging from the code posted, is that he needs to engineer more - a lot more - be it with classes or not. 

Share this post


Link to post
Share on other sites

Thank you very much for your help ;)
I reworked the variables moving most of the "magic numbers" in to const as both of you suggested. 
I also moved the multiple statements per line.

As for the classes I started working on that but as predictable I had trouble doing it now since I would need to redo most of the entire thing.
So now I'm working on a different version for the contest (if I manage to finish it in time) and I'm working with objects from the start.

Here's the upgraded .cpp

 

Pong.cpp

Share this post


Link to post
Share on other sites
23 hours ago, h8CplusplusGuru said:

I disagree, how is adding some classes over-engineering? In this particular case he did not add classes because he does not understand how to use them. There's a lot of code in there that could be a lot more readable and correct with objects, why favor less for the sake of "engineering less"? If there is one thing this fellow needs to learn about, judging from the code posted, is that he needs to engineer more - a lot more - be it with classes or not. 

The whole reason OOP and classes are used is for reusability of code. if he doesn't reuse anything, he shouldn't use classes.  Namespaces are what you use when you want to group similar functions.

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.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!