Breakout-style game

Question

I'm trying to learn how to program with Java which is more or less my first programming language (had some basic Pascal and C,C++ in school and college) but still a Beginner, with a capital 'B'.

Right now I'm working on a mini game, which is similar to the "breakout" game, but it's going to have my own implementation and won't have any blocks to destroy (I know it sounds weird). I have written some core classes, created a field, a pad and a ball. I'd like to get some feedback on my coding: how could I improve my code so it would be easier to implement new features and I'd like to know some of my bad habits that I should try to avoid doing to become good in writing code?

Here are my classes that I have already written:

Game class:

package game;

import java.awt.EventQueue;
import javax.swing.JFrame;

public class Game extends JFrame {

    public Game(){

        Field field = new Field();
        setContentPane(field);

        pack();
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setTitle("BLOCK GAME bETA");
        setVisible(true);


    }

    public static void main(String[] args) {

        EventQueue.invokeLater(() -> {
            Game ex = new Game();
            ex.setVisible(true);
        });
    }

}

Field class:

package game;

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;
import java.awt.event.KeyAdapter;
import java.awt.event.KeyEvent;
import java.util.Timer;
import java.util.TimerTask;
import javax.swing.JLabel;

public class Field extends JLabel{

    //Dimensions of the field. 
    public final int FIELD_HEIGHT = 800;
    public final int FIELD_WIDTH = 480;

    //Creates a pad.
    static Pad pad = new Pad();

    //Creates a ball on top of the pad.
    static Ball ball = new Ball(pad.getPadX(), pad.getPadY());

    private Timer timer;

    //Field constructor, creates a game field.
    public Field(){

        setPreferredSize(new Dimension(FIELD_WIDTH, FIELD_HEIGHT));
        setFocusable(true);

        addKeyListener(new Control());

        timer = new Timer();
        timer.scheduleAtFixedRate(new ScheduleTask(), 100, 20);

    }


    //Draws all of the components on the screen.
    @Override
    public void paintComponent(Graphics g){

        super.paintComponent(g);
        setOpaque(true);
        setBackground(new Color(27, 89, 195));

        drawFieldLimits(g);

        drawPad(g);
        drawBall(g);
    }

    private void drawFieldLimits(Graphics g){

        // 15 pixel offset from the boarder of the frame. 
        g.drawLine(15, 15, 15, FIELD_HEIGHT - 15);
        g.drawLine(15, 15, FIELD_WIDTH - 15, 15);
        g.drawLine(FIELD_WIDTH - 15, 15, FIELD_WIDTH - 15, FIELD_HEIGHT - 15);
    }

    private void drawPad(Graphics g){

        g.fillRect(pad.getPadX(), pad.getPadY(), pad.getPadWidth(), pad.getPadHeight());

    }

    private void drawBall(Graphics g){

        g.fillOval(ball.getBallX(), ball.getBallY(), ball.getBallDiamiter(), ball.getBallDiamiter());

    }

    private class Control extends KeyAdapter{

        @Override
        public void keyReleased(KeyEvent e){
            pad.keyReleased(e);
        }

        @Override
        public void keyPressed(KeyEvent e){
            pad.keyPressed(e);

        }
    }

    private class ScheduleTask extends TimerTask{

        @Override
        public void run() {
            pad.movement(); 
            ball.movement(pad.getPadX(), pad.getPadY());
            repaint();
        }

    }






}

Pad class:

package game;

import java.awt.event.KeyEvent;


public class Pad extends Field{

    private final int PAD_WIDTH = 60;
    private final int PAD_HEIGHT = 10;
    //Starting point of the pad.
    private final int INITIAL_X = (FIELD_WIDTH/2) - (PAD_WIDTH/2);
    private final int INITIAL_Y = FIELD_HEIGHT - PAD_HEIGHT - 10;

    //Starting speed of the pad, may change during the game.
    private int initialPadSpeed = 5;
    private int padSpeed;

    //Pad position.
    private int padX, padY;

    //Constructor
    public Pad(){

        padX = INITIAL_X;
        padY = INITIAL_Y;

    }

    //Defines the movement of the pad.
    public void movement(){

        padX += padSpeed;

        if(padX <= 15){
            padX = 15;
        }

        // 15 is the offset.
        if(padX >= FIELD_WIDTH - PAD_WIDTH - 15){
            padX = FIELD_WIDTH - PAD_WIDTH - 15;
        }

    }


    //Getters.
    public int getPadX(){
        return padX;
    }

    public int getPadY(){
        return padY;
    }

    public int getPadWidth(){
        return PAD_WIDTH;
    }

    public int getPadHeight(){
        return PAD_HEIGHT;
    }

    //Action events used to move the pad left and right.
    public void keyPressed(KeyEvent e) {

        if(e.getKeyCode() == KeyEvent.VK_LEFT){
            padSpeed = -initialPadSpeed;
        }

        if(e.getKeyCode() == KeyEvent.VK_RIGHT){
            padSpeed = initialPadSpeed;
        }
    }

    public void keyReleased(KeyEvent e) {

        if (e.getKeyCode() == KeyEvent.VK_LEFT) {
            padSpeed = 0;
        }

        if (e.getKeyCode() == KeyEvent.VK_RIGHT) {
            padSpeed = 0;
        }

    }


}

Ball class:

package game;

public class Ball extends Pad{

    //Variables that define the ball.
    private int ballX, ballY;
    private int initialBallSpeedVertical = 2;
    private int initialBallSpeedHorizontal = 2;
    private int ballSpeedVertical, ballSpeedHorizontal;
    private int ballDiameter = 10;

    //Constructor that has pad position coordinates as parameters, so the ball will be created on the pad.
    public Ball(int x, int y) {
        ballX = x + 30; // + pad width so the ball is at the middle of the pad.
        ballY = y;

        ballSpeedVertical = initialBallSpeedVertical;
        ballSpeedHorizontal = initialBallSpeedHorizontal;
    }

    //Defines how the ball moves during the game. Consists pad position coordinates
    //so the ball could bounce of the pad.
    public void movement(int padX, int padY){


        ballX -= ballSpeedHorizontal;
        ballY -= ballSpeedVertical;

        if(ballX <= 15){
            ballSpeedHorizontal =-ballSpeedHorizontal;
        }

        if(ballX >= FIELD_WIDTH - ballDiameter - 15){  
            ballSpeedHorizontal = ballSpeedHorizontal =-ballSpeedHorizontal;
        }

        if(ballY <= 15){
            ballSpeedVertical = -ballSpeedVertical;
        }

        if(ballY >= FIELD_HEIGHT - ballDiameter){
            ballSpeedHorizontal = 0;
            ballSpeedVertical = 0;
        }

        //Bouncing of the pad.
        if((ballX >= getPadX() && (ballX <= padX + getPadWidth()) && (ballY >= padY))){
            ballSpeedVertical = -ballSpeedVertical;
        }

    }

    //Getters.
    public int getBallX(){
        return ballX;
    }

    public int getBallY(){
        return ballY;
    }

    public int getBallDiamiter(){
        return ballDiameter;
    }


}

I'd also like to add that I have written this using numerous of online tutorials as an examples and help. I'd also like to know why I get StackOverflowError if I don't assign pad and ball objects as static in Field class because it worked for people who wrote tutorials that I was using?

Also, I know that I could have made some variables final but I think they might be changeable on the future versions of the game (e.g. the ones that determine pad and ball speed).

Also, feel free to bash me for my beginner skills (or lack of them) and don't be afraid of giving me some harsh criticism. Any feedback with coding advice will be much appreciated.

Answer 1

First, the small stuff.

• getBallDiamiter has a misspelling.

• movement kinda sucks as a method name. move would be better, but makes ball.move(30, 100) a bit ambiguous.

• It's better if classes that don't have to be public, aren't. Here, the only thing that needs to be public is Game, as it's your entry point.

• If you have to ask another object for all the info you need to do your job, it shouldn't be your job.

  To that end, the drawPad and drawBall methods in Field shouldn't be there. They should be in Pad and Ball, respectively.

  Once you move them, there's nothing outside of the Ball class that needs to know the ball's position or diameter, and therefore no reason for any of those getters to exist.

• While we're messing around with paintComponent, a more appropriate place to set opacity, background, etc is in the constructor. It won't change unless something else changes it.

• The magic number 15 is everywhere. Looks like it's being used as the game area's margin. Put that number in a constant or something.

• You don't seem to use any functionality specific to JLabel. Pick a more appropriate base class. JPanel works, as by definition it's a generic component.

• Components have a processComponentKeyEvent method, so you don't need to attach a separate key event listener. (On the minus side, the same function processes key-pressed and key-released events as well as key-typed events, so you have to distinguish between them.) Or, if you wanted, you could have Pad implement KeyListener, and add an empty keyReleased method, then the helper class again goes away.

• If you use a javax.swing.Timer or java.util.concurrent.ScheduledExecutorService rather than a java.util.Timer, you get the ability to use a method reference. So you don't need to write a class just to respond to timer ticks.

• Your field and method names shouldn't repeat the class name.

  If you're doing things right, your field and method names don't need to mention the class name. If i'm looking in the Ball class, a field named x should be the ball's X coordinate, and if you insist on a getter for it, it should be named getX.

With that, we get into the elephant in the room here. Yeah, i know why you have getPadX, getBallX, etc, rather than just calling them getX. And that's because

• You're abusing the hell out of inheritance. :P

  Question: Is a Ball a special kind of Pad? Is a Pad a special kind of Field?

  Answer: No, and no. Pad and Ball, you don't even treat as Swing components, let alone labels, let alone game UI areas...and you never treat a Ball like a Pad. If you were to even try, it would look very strange.

  I see why you're inheriting; you want access to FIELD_WIDTH, getPadWidth(), etc. But there are cleaner ways to get access to that info. Ideally, there are ways to remove the need to even know it, and/or move that need to somewhere that can fulfill it better.

  For now, though, the simplest change that would work would be to pass the field to Pad's constructor, and pass the field and pad both to Ball(). It's an ugly band-aid, but eliminates inheritance for Pad and Ball altogether, and opens up a number of other possibilities.

With that fixed, a couple of other things become fixable:

• Static mutable state is the devil. And you only have to do it at all because inheritance breaks otherwise. Once you have everything holding proper references to its dependencies, you can make pad and ball private instance fields.

• You've hard-coded a bunch of things that don't make sense being hard-coded. Again, much of that was entirely to keep inheritance working. But the field, being a Swing component, provides a way to get its actual size...and the pad already provides its own API to do so. While it wasn't as feasible back when everything was shackled together by inheritance, it's downright trivial now. A couple of helper methods, like getLeft() and getRight(), can even take margins into account so that the ball needn't know that margins even exist.