Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This post is a follow-up to When should I consider the number of my method arguments too big?

I took the some advice from previous post and I'm almost done with cleaning up the code. I didn't make any wrapper classes, because my game is already waaaay overkill for this assignment, but I did add more methods and fixed naming.

Are there any rules regarding method order? Like alphabetical, or in order of appearance/usage?

I also need to put comments to my code, because that's what my teacher wants me to do for this assignment. I am not sure how should I comment the code. Is it enough to just put one or two sentences for each method used, or should I describe every variable/group of variables, every import etc?

Also, please tell me what else should I do to clean my code and what did I do wrong.

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.GObject;
import acm.graphics.GOval;
import acm.graphics.GRect;
import acm.program.GraphicsProgram;

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

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

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

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

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

// counters
private static final int PADDING = 10;
private static final int MODIFIER = 3;

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

public void run() {
    addMouseListeners();

    // counters setup
    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();

    // make objects
    GImage paddleLeftTexture = createTexture("texture.png", WIDTH + 1,
            HEIGHT + 1);
    GImage paddleRightTexture = createTexture("texture2.png", WIDTH + 1,
            HEIGHT + 1);
    GImage ballTexture = createTexture("ballTexture.png", (int) BALL_SIZE,
            (int) BALL_SIZE);
    GImage greenFlash = createTexture("greenFlash.png", 100, 300);
    GImage blueFlash = createTexture("blueFlash.png", 100, 300);
    GOval ball = makeBall();
    GRect paddleLeft = makePaddle();
    GRect paddleRight = makePaddle();
    greenFlash.setLocation(-200, 0);
    blueFlash.setLocation(-200, 0);

    // generate GUI
    drawGraphics(ball, paddleLeftTexture, paddleRightTexture, ballTexture,
            greenFlash, blueFlash, counter, paddleLeft, paddleRight,
            aiScore, playerScore);

    // game start
    bounce(labelC, aiScore, playerScore, counter, ball, paddleLeft,
            paddleRight, paddleLeftTexture, paddleRightTexture,
            ballTexture, greenFlash, blueFlash);

}

public void bounce(Color labelC, GLabel aiScore, GLabel playerScore,
        GLabel counter, GOval ball, GRect paddleLeft, GRect paddleRight,
        GImage paddleLeftTexture, GImage paddleRightTexture,
        GImage ballTexture, GImage greenFlash, GImage blueFlash) {

    preGameSetup(ball, paddleRight, paddleRightTexture, counter);
    updateAiScore(aiScore);
    updatePlayerScore(playerScore);
    while (true) {
        moveBall(ballHorizontalSpeed, ballVerticalSpeed, ball, ballTexture);
        movePlayerPaddle(paddleLeft, paddleLeftTexture);
        moveAiPaddle(ball, paddleRight, paddleRightTexture);
        detectHit(ball, paddleRight, paddleLeft, counter, labelC);
        if (TRANSPARENCY >= 0.0f) {
            TRANSPARENCY -= TRANSPARENCY / 100f;
        }
        labelC = new Color(0, 0.0f, 0.0f, TRANSPARENCY);
        counter.setColor(labelC);

        if (detectBallOffScreen(ball)) {
            ballOffScreen(ball, ballTexture, aiScore, playerScore,
                    greenFlash, blueFlash);

            COUNTER = 0;
            bounce(labelC, aiScore, playerScore, counter, ball, paddleLeft,
                    paddleRight, paddleLeftTexture, paddleRightTexture,
                    ballTexture, greenFlash, blueFlash);
        }

        pause(PAUSE);
    }
}

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;

}

private GImage createTexture(String importedImage, int width, int height) {
    Image importResult = getImage(getCodeBase(), importedImage);
    GImage textureResult = new GImage(importResult);
    textureResult.setSize(width, height);
    return textureResult;
}

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

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 detectBallOffScreen(GOval ball) {
    if (ball.getX() < 2*WIDTH - BALL_SIZE || ball.getX() > getWidth() - 2*WIDTH)
        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;
}
/*
private boolean ballHitPaddleBorder(GOval ball, GRect paddle) {
    ;
    if (ball.getX() > paddle.getX() - BALL_SIZE
            && ball.getX() < paddle.getX() + WIDTH
            && ball.getY() > paddle.getY() && ball.getY() < paddle.getY() + ballVerticalSpeed)
        return true;
    else if (ball.getX() > paddle.getX() - BALL_SIZE
            && ball.getX() < paddle.getX() + WIDTH
            && ball.getY() > paddle.getY() + HEIGHT && ball.getY() < paddle.getY() + HEIGHT - ballVerticalSpeed)
        return true;
    else
        return false;
}
*/
private void preGameSetup(GObject ball, GObject paddleRight,
        GObject paddleRightTexture, GLabel counter) {
    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);
    paddleRightTexture.setLocation(getWidth() - 3 * WIDTH, startY - HEIGHT
            / 2);
    paddleRight.setLocation(getWidth() - 3 * WIDTH, startY - HEIGHT / 2);
    paddleRightTexture.sendToFront();
    counter.setLabel(String.valueOf(COUNTER));
    counter.setLocation(getWidth() / 2 - counter.getWidth() / 2,
            getHeight() / 2 + counter.getHeight() / 3.2);
    ballHorizontalSpeed = SPEED * 1.5;
    ballVerticalSpeed = SPEED;
}

    private void updateAiScore(GLabel aiScore) {

    aiScore.setLabel(String.valueOf(AI_SCORE));
    aiScore.setLocation(getWidth() - aiScore.getWidth() - MODIFIER * WIDTH
            - 10, getHeight() - 10);
}

private void updatePlayerScore(GLabel playerScore) {
    playerScore.setLabel(String.valueOf(PLAYER_SCORE));
    playerScore.setLocation(MODIFIER * WIDTH + 10, getHeight() - 10);
}

private void drawGraphics(GObject ball, GObject paddleLeftTexture,
        GObject paddleRightTexture, GObject ballTexture,
        GObject greenFlash, GObject blueFlash, GObject counter,
        GObject paddleLeft, GObject paddleRight, GObject aiScore,
        GObject playerScore) {
    add(ball);
    add(paddleLeftTexture);
    add(paddleRightTexture);
    add(ballTexture);
    add(greenFlash);
    add(blueFlash);
    add(counter);
    add(paddleLeft);
    add(paddleRight);
    add(aiScore);
    add(playerScore);
}

private void detectHit(GOval ball, GRect paddleRight, GRect paddleLeft,
        GLabel counter, Color labelC) {
    if (ballHitBottom(ball) && ballVerticalSpeed >= 0) {
        ballVerticalSpeed *= -1;
    }

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

        ballVerticalSpeed *= -1;
    }

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

    if (ballHitPaddleLeft(ball, paddleLeft)) {
        ballHorizontalSpeed *= -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 (ballHorizontalSpeed > 0)
                ballHorizontalSpeed += 1;
            else
                ballHorizontalSpeed -= 1;
        else if (ballVerticalSpeed > 0)
            ballVerticalSpeed += 0.5;
        else
            ballVerticalSpeed -= 0.5;

    }

/*
    if(ballHitPaddleBorder(ball, paddleLeft)){
        ballVerticalSpeed *= -1;
    }

    if(ballHitPaddleBorder(ball, paddleRight)){
        ballVerticalSpeed *= -1;
    }
*/

}

private void ballOffScreen(GOval ball, GObject ballTexture, GLabel aiScore,
        GLabel playerScore, GObject greenFlash, GObject blueFlash) {
    if (ball.getX() < 2*WIDTH - BALL_SIZE) { // left
        double pos = ball.getY() - greenFlash.getHeight() / 2;
        ballTexture.move(-ballTexture.getWidth()*2, 0);
        AI_SCORE += COUNTER;
        updateAiScore(aiScore);
        for (int i = 20; i < 100; i += 5) {
            greenFlash.setLocation(-i, pos);
            pause(25);
        }
    } else { // right
        double pos = ball.getY() - blueFlash.getHeight() / 2;

        ballTexture.move(ballTexture.getWidth()*2, 0);
        PLAYER_SCORE += COUNTER;
        updatePlayerScore(playerScore);
        for (int i = 20; i < 100; i += 5) {
            blueFlash.setLocation(getWidth() - blueFlash.getWidth() + i,
                    pos);
            pause(25);
        }

    }
}

private void moveBall(double ballHorizontalSpeed, double ballVerticalSpeed,
        GObject ball, GObject ballTexture) {
    ball.move(ballHorizontalSpeed, ballVerticalSpeed);
    ballTexture.setLocation(ball.getX(), ball.getY());
    ballTexture.sendToFront();
}

private void movePlayerPaddle(GObject paddleLeft, GObject paddleLeftTexture) {
    if (mouseY < getHeight() - HEIGHT) { // Player
        paddleLeft.setLocation(2 * WIDTH, mouseY);
        paddleLeftTexture.setLocation(2 * WIDTH, mouseY);
        paddleLeftTexture.sendToFront();
    } else {
        paddleLeft.setLocation(2 * WIDTH, getHeight() - HEIGHT);
        paddleLeftTexture.setLocation(2 * WIDTH, getHeight() - HEIGHT);
        paddleLeftTexture.sendToFront();

    }
}

private void moveAiPaddle(GOval ball, GRect paddleRight,
        GImage paddleRightTexture) {
    if (AI_GODMODE == true) {
        if (ball.getY() < getHeight() - HEIGHT / 2
                && ball.getY() > HEIGHT / 2) {
            paddleRight.setLocation(getWidth() - 3 * WIDTH, ball.getY()
                    - HEIGHT / 2);
            paddleRightTexture.setLocation(getWidth() - 3 * WIDTH,
                    ball.getY() - HEIGHT / 2);
            paddleRightTexture.sendToFront();
        } else if (ball.getY() <= HEIGHT / 2) {
            paddleRight.setLocation(getWidth() - 3 * WIDTH, 0);
            paddleRightTexture.setLocation(getWidth() - 3 * WIDTH, -0);
            paddleRightTexture.sendToFront();
        } else {
            paddleRight.setLocation(getWidth() - 3 * WIDTH, getHeight()
                    - HEIGHT);
            paddleRightTexture.setLocation(getWidth() - 3 * WIDTH,
                    getHeight() - HEIGHT);
            paddleRightTexture.sendToFront();
        }
    } else { // end godMode if
        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);
                paddleRightTexture.move(0, -AI_SPEED);

            } else if (targetY > paddleRight.getY() + HEIGHT / 2) {
                paddleRight.move(0, AI_SPEED);
                paddleRightTexture.move(0, AI_SPEED);
            }
        } // end normalMode if
    } // end modeSelector if
} // end moveAiPaddle void
} // end class

Part of the code is not working properly yet, so I put it in comment /* [...] */

EDIT:

Updated updateScores(); method

share|improve this question

2 Answers 2

up vote 7 down vote accepted

As you noticed: it's hard to force comments when the code is written well. Once you get rid of magic values, create descriptive names for both variables and methods and make sure each method does one thing you have basically reached a so-called self-documenting code.

And that's good: this is the kind of code you want to reach. However, teachers are teachers and they won't always recognize your awesomeness.

Luckily for you there are two main categories of commenting.

Many new programmers comment the what: "what does the code do?". These are very uninteresting comments, an example could be this:

// In case the mouse moved
public void mouseMoved(MouseEvent e) {
    mouseY = e.getY(); // Update the mouse's Y-position
}

These type of comments just reiterate what we can already read very clearly from the code itself (see: self-documenting code).

The other type of comments elaborate on the why. These are comments that explain your reasoning behind a bit of code, something that is a lot harder to express in code.

Take for example this piece of code:

private void updatePlayerScore(GLabel playerScore) {
    playerScore.setLabel(String.valueOf(PLAYER_SCORE));
    playerScore.setLocation(MODIFIER * WIDTH + 10, getHeight() - 10);
}

When I read this, I ask myself the question "Why did the location of that score get changed?". This is an excellent opportunity for you to add a comment why that is:

// Change location because the playfield changed size
private void updatePlayerScore(GLabel playerScore) {
    playerScore.setLabel(String.valueOf(PLAYER_SCORE));
    playerScore.setLocation(MODIFIER * WIDTH + 10, getHeight() - 10);
}

Now I will immediately know what your intentions were and the general idea behind your approach is clearer. I'm certain your teachers will appreciate such informative commenting when they have to look through a multitude of assignments, many which will not have gone through a review.

Member order

Regarding your edit on method order (which I changed to member order to get some other things as well): usually I would point you to the Java Code Conventions but appearantly they all give a 404 right now. However from the top of my head, this is the order of which a class' members should appear:

  • Constants
  • Static variables
  • Instance variables
  • Static constructor
  • Initializer block
  • Constructor
  • Methods

The actual order of the content of each type is not important (except variables that use another variables: they have to be defined in the right order).

Sidenotes

Just to remark a few other things:

  • Still some magic values left at the bottom of the file
  • Many public variables. Some might be intended like that, but your Random instance shouldn't be amongst them.
share|improve this answer
    
I found some more info regarding order in 5.2.40 here javastyle.html As for magic numbers, I don't need to put 2 into variable when calculating half of height, right? This seems to be the decreasing code readability. I have one more random number which I don't know what to do with, it's 3.2 used to align one of the counters. Text label does not align horizontally the same way i did verically (half total - half text size), i got it with trial and error. –  Asalas77 Apr 5 at 12:58
    
Oh, if that is the reason of 2 then you can leave it as it is. Although I might just extract all these movements into their own methods like moveDown(GRect), but that's it. That 3.2 variable is an excellent opportunity to add a comment! –  Jeroen Vannevel Apr 5 at 13:07

I don't know Java, but I can tell you that in the first /* ... / section, both if statements are the same. Also, in the second / ... */ section, rather than having 2 if's, just say

if((firstifcondition) OR (secondifcondition))
{
      ...
}
share|improve this answer
    
There is a slight difference in if statements, on Y axis are is + HEIGHT in the second one –  Asalas77 Apr 5 at 11:18

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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