Simple Pong game

Question

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");
    AiScore.setFont("Impact-100");
    PlayerScore.setFont("Impact-100");
    Color labelC = new Color(0, 0.0f, 0.0f, TRANSPARENCY);
    Color scoreC = new Color(0, 0.0f, 0.0f, 0.1f);
    counter.setColor(labelC);
    AiScore.setColor(scoreC);
    PlayerScore.setColor(scoreC);
    counter.setLocation(getWidth() / 2 - counter.getWidth() / 2,
            getHeight() / 2 + counter.getHeight() / 3.2);
    counter.sendToFront();
    Image texture = getImage(getCodeBase(), "texture.png");
    Image texture2 = getImage(getCodeBase(), "texture2.png");
    Image ballTexture = getImage(getCodeBase(), "ballTexture.png");
    Image greenFlash = getImage(getCodeBase(), "greenFlash.png");
    Image blueFlash = getImage(getCodeBase(), "blueFlash.png");
    GImage image = new GImage(texture);
    GImage image2 = new GImage(texture2);
    GImage image3 = new GImage(ballTexture);
    GImage image4 = new GImage(greenFlash, -250, 0);
    GImage image5 = new GImage(blueFlash, -250, 0);
    add(image);
    add(image2);
    add(image3);
    add(image4);
    add(image5);
    add(counter);
    image.setSize(WIDTH + 1, HEIGHT + 1);
    image2.setSize(WIDTH + 1, HEIGHT + 1);
    image3.setSize(BALL_SIZE, BALL_SIZE);
    image4.setSize(100, 300);
    image5.setSize(100, 300);
    GOval ball = makeBall();
    GRect paddleLeft = makePaddle();
    GRect paddleRight = makePaddle();
    add(paddleLeft);
    add(paddleRight);
    bounce(labelC, AiScore, PlayerScore, counter, ball, paddleLeft,
            paddleRight, image, image2, image3, image4, image5);

}

public static GRect makePaddle() {
    GRect result = new GRect(0, 0, WIDTH, HEIGHT);
    result.setFilled(true);
    result.setColor(Color.BLACK);
    return result;
}

public static GOval makeBall() {
    GOval result = new GOval(150, 100, BALL_SIZE, BALL_SIZE);
    result.setFilled(true);
    result.setColor(Color.WHITE);
    return result;

}

public void mouseMoved(MouseEvent e) {
    mouseY = e.getY();
}

public void bounce(Color labelC, GLabel AiScore, GLabel PlayerScore,
        GLabel counter, GOval ball, GRect paddleLeft, GRect paddleRight,
        GImage image, GImage image2, GImage ballTexture, GImage greenFlash,
        GImage blueFlash) {
    add(ball);
    add(ballTexture);
    add(AiScore);
    add(PlayerScore);
    PlayerScore.setLabel(String.valueOf(PLAYER_SCORE));
    PlayerScore.setLocation(3*WIDTH+10,getHeight()-10);
    AiScore.setLabel(String.valueOf(AI_SCORE));
    AiScore.setLocation(getWidth()-AiScore.getWidth()-3*WIDTH-10,   getHeight()-10);
    startX = rand.nextInt((int) (getWidth() * 0.8))
            + (int) (0.1 * getWidth()); // zapobiega pojawieniu się piłki po
                                        // lewej stronie lewej paletki
    startY = rand.nextInt(getHeight());
    ball.setLocation(startX, startY);
    image2.setLocation(getWidth() - 3 * WIDTH, startY - HEIGHT / 2);
    paddleRight.setLocation(getWidth() - 3 * WIDTH, startY - HEIGHT / 2);
    image2.sendToFront();
    counter.setLabel(String.valueOf(COUNTER));
    counter.setLocation(getWidth() / 2 - counter.getWidth() / 2,
            getHeight() / 2 + counter.getHeight() / 3.2);
    dx = SPEED * 1.5;
    dy = SPEED;
    while (true) {

        ball.move(dx, dy);
        ballTexture.setLocation(ball.getX(), ball.getY());
        ballTexture.sendToFront();
        if (TRANSPARENCY >= 0.0f)
            TRANSPARENCY -= TRANSPARENCY / 100f;

        labelC = new Color(0, 0.0f, 0.0f, TRANSPARENCY);
        counter.setColor(labelC);
        if (mouseY < getHeight() - HEIGHT) { // Player
            paddleLeft.setLocation(2 * WIDTH, mouseY);
            image.setLocation(2 * WIDTH, mouseY);
            image.sendToFront();
        } else {
            paddleLeft.setLocation(2 * WIDTH, getHeight() - HEIGHT);
            image.setLocation(2 * WIDTH, getHeight() - HEIGHT);
            image.sendToFront();

        }

        // AI, z którym da się wygrać
/*
        double targetY = ball.getY() + BALL_SIZE / 2;
        if (targetY < getHeight() - HEIGHT / 2 && targetY > HEIGHT / 2) {
            if (targetY < paddleRight.getY() + HEIGHT / 2) {
                paddleRight.move(0, -AI_SPEED);
                image2.move(0, -AI_SPEED);

            } else if (targetY > paddleRight.getY() + HEIGHT / 2) {
                paddleRight.move(0, AI_SPEED);
                image2.move(0, AI_SPEED);

            }
        }
*/
        // AI, z którym nie da się wygrać
        // Zamiennie z kodem powyżej
        // Jeden z algorytmów musi być w komentarzu: /* .: code :. */



        if (ball.getY() < getHeight() - HEIGHT / 2
                && ball.getY() > HEIGHT / 2) { // AI
            paddleRight.setLocation(getWidth() - 3 * WIDTH, ball.getY()
                    - HEIGHT / 2);
            image2.setLocation(getWidth() - 3 * WIDTH, ball.getY() - HEIGHT
                    / 2);
            image2.sendToFront();
        } else if (ball.getY() <= HEIGHT / 2) {
            paddleRight.setLocation(getWidth() - 3 * WIDTH, 0);
            image2.setLocation(getWidth() - 3 * WIDTH, -0);
            image2.sendToFront();
        } else {
            paddleRight.setLocation(getWidth() - 3 * WIDTH, getHeight()
                    - HEIGHT);
            image2.setLocation(getWidth() - 3 * WIDTH, getHeight() - HEIGHT);
            image2.sendToFront();
        }

        if (ballHitBottom(ball) && dy >= 0) {
            dy *= -1;
        }

        if (ballHitTop(ball) && dy <= 0) {

            dy *= -1;
        }

        if (ballHitPaddleRight(ball, paddleRight)) {
            dx *= -1;
        }

        if (ballHitPaddleLeft(ball, paddleLeft)) {
            dx *= -1;
            COUNTER++;
            counter.setLabel(String.valueOf(COUNTER));
            counter.setLocation(getWidth() / 2 - counter.getWidth() / 2,
                    getHeight() / 2 + counter.getHeight() / 3.2);
            TRANSPARENCY = 0.1f;
            labelC = new Color(0, 0.0f, 0.0f, TRANSPARENCY);
            counter.setColor(labelC);

            boolean bool = rand.nextBoolean();
            if (bool)
                if (dx > 0)
                    dx += 1;
                else
                    dx -= 1;
            else if (dy > 0)
                dy += 0.5;
            else
                dy -= 0.5;

        }
        pause(PAUSE);


        if (ballOffScreen(ball)) {
            if (ball.getX() + SPEED * 2 < 0) { // left
                double pos = ball.getY() - greenFlash.getHeight() / 2;
                remove(ball);
                remove(ballTexture);
                AI_SCORE+=COUNTER;
                AiScore.setLabel(String.valueOf(AI_SCORE));
                AiScore.setLocation(getWidth()-AiScore.getWidth()-3*WIDTH-10,   getHeight()-10);
                for (int i = 20; i < 100; i += 5) {
                    greenFlash.setLocation(-i, pos);
                    pause(25);
                }
            } else { // right
                double pos = ball.getY() - blueFlash.getHeight() / 2;
                remove(ball);
                remove(ballTexture);
                PLAYER_SCORE+=COUNTER;
                PlayerScore.setLabel(String.valueOf(PLAYER_SCORE));
                PlayerScore.setLocation(10+3*WIDTH,getHeight()-10);
                for (int i = 20; i < 100; i += 5) {
                    blueFlash.setLocation(getWidth() - blueFlash.getWidth()
                            + i, pos);
                    pause(25);
                }

            }

            COUNTER = 0;
            bounce(labelC, AiScore, PlayerScore, counter, ball, paddleLeft,
                    paddleRight, image, image2, ballTexture, greenFlash,
                    blueFlash);
        }
    }
}

private boolean ballHitBottom(GOval ball) {
    double bottomY = ball.getY() + ball.getHeight();
    return bottomY >= getHeight();
}

private boolean ballHitTop(GOval ball) {
    double topY = ball.getY();
    return topY <= 0;
}

private boolean ballHitPaddleRight(GOval ball, GRect paddle) {
    double rightX = ball.getX() + ball.getWidth();
    double rightY = ball.getY() + ball.getHeight() / 2;
    double paddlePosX = paddle.getX();
    double paddlePosY = paddle.getY();
    if (rightX > paddlePosX && rightY > paddlePosY
            && rightY < paddlePosY + paddle.getHeight())
        return true;
    else
        return false;
}

private boolean ballOffScreen(GOval ball) {
    if (ball.getX() + SPEED * 2 < 0
            || ball.getX() + ball.getWidth() - SPEED * 2 > getWidth())
        return true;
    else
        return false;
}

private boolean ballHitPaddleLeft(GOval ball, GRect paddle) {
    double leftX = ball.getX();
    double leftY = ball.getY();
    double paddlePosX = paddle.getX() + WIDTH;
    double paddlePosY = paddle.getY();
    if (leftX < paddlePosX && leftY > paddlePosY
            && leftY < paddlePosY + paddle.getHeight())
        return true;
    else
        return false;
}
}

Jeroen Vannevel · Accepted Answer · 2014-04-04 18:52:34Z

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 < 0
        || ball.getX() + ball.getWidth() - 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 < 0 || ball.getX() + ball.getWidth() - 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 < 0;
boolean ballIsThick = ball.getX() + ball.getWidth() - SPEED * 2 > getWidth();

return ballIsFast || ballIsThick;

+1, but I actually tend to disagree with "always add brackets to if statements". It often makes for much cleaner code to use in-line ifs when they're very short. — asteri, Commented Apr 4, 2014 at 19:08
@Asalas77: yes, that's pretty much exactly it. One remark though, an instance is created like this: GameParameters gamepar = new GameParameters(); (notice the empty brackets to signify a parameterless constructor). You should indeed look into creating separate classes if you want to do it in OO style but OO isn't always the best solution either so perhaps there are better (easier) projects to learn OO in. — Jeroen Vannevel, Commented Apr 4, 2014 at 19:16
No, you will never notice a difference in performance by storing a value in a variable. Definitely don't hold back on that. If the result from getHeight() is always the same then you could, but that's up to yourself. If you use it multiple times then it might be clearer to do so, it's definitely okay. — Jeroen Vannevel, Commented Apr 4, 2014 at 19:28
@Asalas77: it would simply change your method signature to bounce(GameParameters params) where you can now use those parameters as params.playerScore. When you need those parameters elsewhere, you just pass your params object to it. As to your second question: that would make sense, but I'm not sure if that's also appropriate when you're working with a local variable instead of an instance variable. I think both are correct for local variables (in a method), but you're right if you mean instance ones. Remember to declare them private static final — Jeroen Vannevel, Commented Apr 4, 2014 at 21:22
+1 I would only add that sometimes domain-specific names are omay: in math and graphics, dx is commonly understood as delta-x. Short names make complicated formulas easier to read as long as there underlying meaning is clear to the reader. Anyone maintaining this code should have this basic understanding, so the chance of confusion is low. — David Harkness, Commented Apr 5, 2014 at 2:30

Stack Exchange Network

Simple Pong game

1 Answer 1

Naming

Magic numbers

Spacing

Split up your code

Brackets & shortcuts

Your Answer

Not the answer you're looking for? Browse other questions tagged
java
game
animation
or ask your own question.

Linked

Hot Network Questions

Simple Pong game

1 Answer 1

Naming

Magic numbers

Spacing

Split up your code

Brackets & shortcuts

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javagameanimation or ask your own question.

Linked

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
java
game
animation
or ask your own question.