If you would prefer to post the code directly here please care to mention it so I can add it as fast as possible.

I have some comments to make, things I think will make the code more readable.

1 - You don't need to comment things that are auto-commented. What I mean is, things like:

//The font in use
TTF_Font *font;

...

//The collision sound
Mix_Chunk *collisionSound;

don't need those comments.

Something that could use some comments is this line in globals.h:

const int SPEED = 1000;

What does it mean that speed is one thousand? Should that be used as pixels per second or something else? Adding comments there will not only help on readability, it'll also make the code more coherent, since now you must think what it means and check if that makes sense ("It means 1000 pixels... but wait, that's not a speed unit, am I using it right?").

2 - When you quit the program for some error you should return a different value. When init() and load_files() fail you return 1 for both, it's good to return consecutive numbers so you know what failed without debugging.

3 - Check that you're using the variables for what they where created, or check that the names match their purpose. Look at this:

if(win == false)//Play button
{
 ...
}
else if(win == true)//Play again button
{
 ...
}

The "win" boolean looks like it should be used to flag when the player won, but you're using it to detect what button was pressed in the menu.

Also, in an if/else where the condition is just a boolean value you don't need and "else if" with the opposite condition. if the code reached the "else" block the win variable is already true. Probably a compiler will ignore that second if, but when reading the code it could create some confusion.

I'm just guessing here, but if you did this to avoid using another boolean, it'll be better to use another boolean. Reusing variables for other purposes can be the cause of big problems in bigger projects.

4 - This may have more implications than just readability. In one place you do:

player1.move(delta.get_ticks());
player2.move(delta.get_ticks());

If you meant to send the same "ticks" value to both players you should create a temp variable and use that. get_ticks() might return a differente value the second time you call it. Maybe it doesn't matter in this case, but it's good to check.

5 - When you set the "score" text your checking for each possible value in an if/else if/else if chain. This has 2 problems:

- If you decide that 3 is too low and want a max score of 10 you'll have to add more lines.

- All blocks do the same, so if you want to change something (add a "!" after the score, for example) you have to add that "!" in every block.

So, to fix this you can create a const char * from the int without knowing the value, and use that const char * as the second parameter. There are more than one way to do it, check some of them here: http://stackoverflow.com/questions/7934907/convert-int-to-const-char-in-order-to-write-on-file

6 - You have lots of variables in some classes, does the "Ball" class really need access to all the surfaces?

7 - Also, you have some magic numbers in some places that might not work if you change, for example, the screen size. Maybe you can change those values to some percentage of the screen size.

Thanks for the tips my friends. I will find the time soon ,because of finals period, and correct my mistakes and will report back here in the comments with the actual changes. I really appreciate your help !

I have some comments to make, things I think will make the code more readable.

1 - You don't need to comment things that are auto-commented. What I mean is, things like:

//The font in use
TTF_Font *font;

...

//The collision sound
Mix_Chunk *collisionSound;

don't need those comments.

Something that could use some comments is this line in globals.h:

const int SPEED = 1000;

What does it mean that speed is one thousand? Should that be used as pixels per second or something else? Adding comments there will not only help on readability, it'll also make the code more coherent, since now you must think what it means and check if that makes sense ("It means 1000 pixels... but wait, that's not a speed unit, am I using it right?").

2 - When you quit the program for some error you should return a different value. When init() and load_files() fail you return 1 for both, it's good to return consecutive numbers so you know what failed without debugging.

3 - Check that you're using the variables for what they where created, or check that the names match their purpose. Look at this:

if(win == false)//Play button
{
 ...
}
else if(win == true)//Play again button
{
 ...
}

The "win" boolean looks like it should be used to flag when the player won, but you're using it to detect what button was pressed in the menu.

Also, in an if/else where the condition is just a boolean value you don't need and "else if" with the opposite condition. if the code reached the "else" block the win variable is already true. Probably a compiler will ignore that second if, but when reading the code it could create some confusion.

I'm just guessing here, but if you did this to avoid using another boolean, it'll be better to use another boolean. Reusing variables for other purposes can be the cause of big problems in bigger projects.

4 - This may have more implications than just readability. In one place you do:

player1.move(delta.get_ticks());
player2.move(delta.get_ticks());

If you meant to send the same "ticks" value to both players you should create a temp variable and use that. get_ticks() might return a differente value the second time you call it. Maybe it doesn't matter in this case, but it's good to check.

5 - When you set the "score" text your checking for each possible value in an if/else if/else if chain. This has 2 problems:

- If you decide that 3 is too low and want a max score of 10 you'll have to add more lines.

- All blocks do the same, so if you want to change something (add a "!" after the score, for example) you have to add that "!" in every block.

So, to fix this you can create a const char * from the int without knowing the value, and use that const char * as the second parameter. There are more than one way to do it, check some of them here: http://stackoverflow.com/questions/7934907/convert-int-to-const-char-in-order-to-write-on-file

6 - You have lots of variables in some classes, does the "Ball" class really need access to all the surfaces?

7 - Also, you have some magic numbers in some places that might not work if you change, for example, the screen size. Maybe you can change those values to some percentage of the screen size.

Okay then. The github repository is update now and I tried to follow your tips. So...

1)I know some of the variables are self commented but I personally like them paired with a comment too,at least now in this project and as a beginner I will keep them.In addition about the speed variable it is supposed to be pixels but I need to rework the whole moving system of the paddles and the physics in general(probably need to follow a more scientific method as in just implementing all calculations a more formal way since the one I used is from my observations.If you have a tutorial,snippet,book to recommend for that please care to do so.

2)Good point ! I think I fixed that.

3)Well I didn't commented good enough this line of code so that's why you misunderstood the meaning of it. I think now it's more clear.

4)Very good point and also I fixed it.

5)Done.

6)Well I have a problem in distinguishing what needs to stay and what needs to leave, really any guidelines will really help me.

7)That needs to be fixed too as a screen-dimension independent game is sure a thing so I will look into it at some point.

You don't need a "RightPaddle" and "LeftPaddle" class. Make a "Paddle" class, and pass in the parameters to the constructor that defines where it lives, and what keys move it.

Also, you shouldn't blindly have every surface defined in "globals.h" and have every class have access to them. Define those surfaces in the classes that use them, or in the main file where they're used. Then, you can pass in what information each class needs.

The huge if else if block in main.ccp is not needed. Simply do something like this:

char score[4];
 
// get player 1 score
sprintf(score, "%d", ball.Player1Score);
score1 = TTF_RenderText_Solid(font, score, textColor);
 
// get player 2 score
sprintf(score, "%d", ball.Player2Score);
score2 = TTF_RenderText_Solid(font, score, textColor);

And, why are score1 and score2 globals? Shouldn't they should be defined in main() since that's the only place they are used?

I didn't look at the rest, just a few things I noticed.

Good luck and have fun.

So....your observations were on point my friend so I implemented your tips about the Paddle class and the score printing method. The thing left as I mentioned above is that I really can't figure how to organize my global variables and what needs to stay or leave.

Also do you think that I should move onto a next simple game because I don't think optimizing every bits of this pong clone gonna give me anything more, apart from a slightly more elegant collision detection and physics system.

At last thank you very much for taking the time to help me with your tips,observations and suggestions ! If you have anything else that you don't like/boggles or just general help please care to mention it !

Okay then. The github repository is update now and I tried to follow your tips. So...

1)I know some of the variables are self commented but I personally like them paired with a comment too,at least now in this project and as a beginner I will keep them.In addition about the speed variable it is supposed to be pixels but I need to rework the whole moving system of the paddles and the physics in general(probably need to follow a more scientific method as in just implementing all calculations a more formal way since the one I used is from my observations.If you have a tutorial,snippet,book to recommend for that please care to do so.

2)Good point ! I think I fixed that.

3)Well I didn't commented good enough this line of code so that's why you misunderstood the meaning of it. I think now it's more clear.

4)Very good point and also I fixed it.

5)Done.

6)Well I have a problem in distinguishing what needs to stay and what needs to leave, really any guidelines will really help me.

7)That needs to be fixed too as a screen-dimension independent game is sure a thing so I will look into it at some point.

1 - I'm not sure of a good source, but if you have just a constant velocity when moving you only need one function (x += speed * deltaTime) and set speed to a positive or negative value depending on the direction the paddle should move.

2 - That's good, but you still have a return 1 at the end of the while loop in the main function, when the SDL_Flip fails.

3 - Ok, I get it now, great.

6 - I don't know of a guideline, I do this things automatically, but some ideas that come to mind are:

- Constants are better candidates for globals than dynamic things.

- Functions that are called only once are rarely candidates for constants, you can define them directly over the "main" code since you won't call them anymore. Even functions that are called more often look weird as global. Think about using some utilitary class with static methods instead.

- If you don't use something, don't provide access. In your Ball class you defined all the SDL_Surfaces but you only use "ball" and "screen". This gives you a hint that the other surfaces maybe are not good as globals.

- If something is related to an object that has a class, probably that global belongs to that class instead. PADDLE_HEIGHT and PADDLE_WIDTH could be static constant members of the Paddle class, and you can then access those values using Paddle::PADDLE_HEIGHT for instance. The same for the ball size and speed, they could be defined in the Ball class. Even some surfaces may be better inside the class definition. The ball SDL_Surface is only used on the "show" method? Then it could be a private member of the Ball class, initialized on the constructor.

- If you need something only in one place on each class maybe it shouldn't be global, since globals give access to it everywhere. The screen SDL_Surface is only used on the "show" methods of the other classes, so you can remove it from globals.h and send it as a parameter to the "show" methods (this is really common in most API's, you send the renderer/surface/whatever-it's-called to the display/show functions).