Simple Pong game

I'm writing a simple Pong game. I have a method bounce() that makes everything run: the ball, the paddles, score etc. and it uses 12 arguments: 3 counters, 2 paddles, ball, 5 textures, 1 color. I'm a beginner to programming and only have couple months college experience so I don't know if that's too much. In classes, I usually had 1-3 arguments per method. Am I doing something wrong?

In method run() I'm creating everything I need for the graphics, then I call looped bounce() and I need to pass everything I just created as an argument. This seems very wrong to me, but I fail to find a way around it.

Please ignore non-English comments in code.

```
import java.awt.Color;
import java.awt.Image;
import java.awt.event.MouseEvent;
import java.util.Random;

import acm.graphics.GImage;
import acm.graphics.GLabel;
import acm.graphics.GOval;
import acm.graphics.GRect;
import acm.program.GraphicsProgram;

/* TO DO LIST
* ------------------
* Corner Bounce
* Difficulty Level
*
*
*
*/

@SuppressWarnings("serial")
public class Pong extends GraphicsProgram {
private static final double PAUSE = 1000 / 96.0;
public Random rand = new Random();
public double mouseY;

// ball
private static final double BALL_SIZE = 20;
private static final double SPEED = 5;
public double dx = SPEED * 1.5;
public double dy = SPEED;
public double startX;
public double startY;

// paddle
private static double HEIGHT = 150;
private static double WIDTH = 15;
private static int COUNTER = 0;
public static double AI_SPEED = 9.0; // difficulty 0-20

// label
private float TRANSPARENCY = 0.0f;
public int AI_SCORE = 0;
public int PLAYER_SCORE = 0;

public static void main(String[] args) {
Pong p = new Pong();
p.run();
}

public void run() {
addMouseListeners();
GLabel counter = new GLabel(String.valueOf(COUNTER));
GLabel AiScore = new GLabel(String.valueOf(AI_SCORE));
GLabel PlayerScore = new GLabel(String.valueOf(COUNTER));
counter.setFont("Impact-600

Glad to see you made it here.

There is no predefined number of "this many arguments and no further" so it is a subjective decision. In my experience, I would say that as soon as you see more than 5 arguments you have to be pretty sure that that is a good way to go.

A common approach here is to create a wrapper class that just holds these variables and which can subsequently be used to keep your method arguments clean, easily adaptable (adding another variable is a matter of adding it to your class) and it's also cleaner when you have to pass it to multiple methods.

A sample of how this might look:

class GameParameters {
    public Color labelC;
    public GLabel playerScore;
    public GImage blueFlash;
}

Note that I am using public variables so you don't have needless clutter of getters/setters. This class is just a wrapper to send values, nothing else.

Some notes, perhaps?

Naming

In that same method you have the following parameter names:

• image

• image2

• AiScore

• PlayerScore

The former two are rather unclear: what is the purpose of these two images? Naming a variable by its purpose will be a lot clearer than "image" and "image2".

The latter two don't follow the lowerCamelCase convention style which (as you can see) meddles with syntax markup.

Also in the category of meaningful names: dx and dy are seldom meaningful. Consider making them more descriptive.

Magic numbers

Take for example this snippet:

PlayerScore.setLocation(3*WIDTH+10,getHeight()-10);

What are 3 and 10? Magic numbers are values which look like they're being put there randomly without any context as to what they mean.

The solution to this is to put these values into variables which will make their intention clear:

int multiplier = 3;
int padding = 10;

PlayerScore.setLocation(multiplier * WIDTH + padding, getHeight() - padding);

This will also have the side-effect of making sure you don't accidentally type 10 and 100 since everything is stored in one place.

Spacing

Keep your spacing consistent! In a short timespan I see these three lines:

PlayerScore.setLocation(3*WIDTH+10,getHeight()-10);
AiScore.setLocation(getWidth()-AiScore.getWidth()-3*WIDTH-10,   getHeight()-10);
image2.setLocation(getWidth() - 3 * WIDTH, startY - HEIGHT / 2);

Ideally everything should be spaced out the way the last one is: spaces between the individual operators, none right before the comma, none right next to the parentheses.

Split up your code

Your bounce() method is very big and looks like it does more than one thing. Consider extracting your boundary checks into a different method to keep the overview.

Brackets & shortcuts

Consider this code:

if (ball.getX() + SPEED * 2  getWidth())
    return true;
else
    return false;

Always add brackets ({}) to your if statements, it doesn't matter how short the body is. If you don't, you'll guaranteed end up with a logical error when you suddenly decide to add another line to the body.

However, in this case you might as well write

return ball.getX() + SPEED * 2  getWidth();

Although you could, for clarity purposes, also store the two conditions into variables (my variable names are examples).

boolean ballIsFast = ball.getX() + SPEED * 2  getWidth();

return ballIsFast || ballIsThick;

StackExchange Code Review Q#46310, answer score: 12