Java Pong implementation

Question

I was making a pong game when I realized (by looking at other people's examples) that my Pong code was very messy and not organized. I would like to ask for assistance.

import java.awt.*;
import java.awt.Component;
import java.awt.event.*;
import javax.swing.*;
import javax.swing.Timer;

public class Pong {

  JFrame frame;
  //JButton up;
  //JButton down;
  JTextField controls;
  MyDrawPanel drawPanel;
  int y = 325;
  int edge;
  int bx = 650;
  int by = 325;
  boolean gameOn = true;

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

  public void go() {

    frame = new JFrame();
  //  up = new JButton();
    //down = new JButton();
    controls = new JTextField();
    drawPanel = new MyDrawPanel();

    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.add(controls);
    controls.requestFocus();
    controls.addKeyListener(new keyListener());
    frame.setTitle("Pong");
    frame.setSize(1366,768);
    frame.getContentPane().setBackground(Color.black);

  /*  up.setText("UP");
    up.setBackground(Color.white);
    up.addActionListener(new upListener());
    down.setText("DOWN");
    down.setBackground(Color.white);
    down.addActionListener(new downListener());

    frame.getContentPane().add(BorderLayout.SOUTH, down);
    frame.getContentPane().add(BorderLayout.NORTH, up); */
    frame.getContentPane().add(drawPanel);

    frame.setVisible(true);

    while(gameOn) {

    }
  }

  class keyListener implements KeyListener {
    public void keyPressed(KeyEvent e) {
    //  int id = e.getID();
      char key = e.getKeyChar();
      //System.out.println("Key Pressed" + " " + key);

      if(key == 'w' && y > 0) {
        y = y-4;
        drawPanel.repaint();
        edge = 0;
      }

      else if(key == 's' && y < 560) {
        y = y+4;
        drawPanel.repaint();
        edge = 0;
      } else {
        if(edge < 3) {
        System.out.println("edge");
        edge++;
       }
      }
    }

    public void keyReleased(KeyEvent e) {

    }

    public void keyTyped(KeyEvent e) {

    }
  }


/*  class upListener implements ActionListener {
    public void actionPerformed(ActionEvent event) {
      System.out.println("left");
      for(int i = 20; i > 0; i--) {
        Timer timer = new Timer(5, this);
      //  timer.setInitialDelay(19000);
        timer.start();
        y--;
        drawPanel.repaint();
      }
    }
  }
  class downListener implements ActionListener {
    public void actionPerformed(ActionEvent event) {
      System.out.println("right");
      y = y + 10;
      drawPanel.repaint();
    }
  } */
  class MyDrawPanel extends JPanel {

    public void paintComponent(Graphics g) {
      g.setColor(Color.black);
      g.fillRect(0,0,this.getWidth(),this.getHeight());

      g.setColor(Color.white);
      g.fillRect(1300,y,30,150);

      g.setColor(Color.white);
      g.fillOval(bx,by,20,20);
    }
  }
}

Answer 1

Thanks for sharing your code.

Naming

Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

Naming Conventions

Please read and follow the Java Naming Conventions

eg.:

• Let class names always start with an uppercase letter
• let boolean variables and methods returning a boolean value start with is, has, can or alike. (isGameRunning vs. gameOn)

Choose you names from the problem domain, not from the technical solution.

eg.: your class keyListener might better be named PadelControl

don't use comments to "save" code

Modern IDEs keep a history of the last saved versions of your code files. They have capability to compare with this old versions and get them back (or parts of). Also using a source code management system like git or svn is quite easy for the same purpose.

So there is no need to keep "inactive code" as comments in your code files. just remove them.

use adapter classes if possible

Almost all Swing interfaces have empty implementation classes called *Adapter in your case it is the KeyAdapter. You can extend that class and only implement the method you need, keeping your code clean from the empty implementation of the unneeded methods.

do not add code with unknown/unverified behavior

You method go() ends with an empty loop. I suspect you added this because you thought you program would exit otherwise. Maybe you have been told to do so or red somewhere. But you most likely never tested that behavior.

This empty loop results in 100% activity of one of your CPUs cores. Some years ago, when there was only one core your computer would have been unusable after the start of your program. Thanks to technology advance this loop does not stall your PC.

So never add code just because it sounds like a good idea, always test that certain code really has the expected effect. If it doesn't delete it immediately.

There is a programing technique called Test Driven Development supporting that.

Answer 2

1. This isn't really Pong yet. Your program runs but there is no code to make the ball move. If you just want help at the point of making the paddle go up and down, then say that in your question.
2. You will notice that other Pong programs separate the ball and the paddle out as separate classes. This will help you organize your code better as you add features. For example, when you get the ball moving, the code to determine a Paddle - Ball collision can be much cleaner if paddle and ball are separate classes. Also, when you want to add another player, it will be easy to throw another object of class "Paddle" on the screen than it would be to write it all out in the Panel class, duplicating some of the code for the second Paddle.
3. Like Timothy Truckle said, get rid of those commented lines and the empty loop.
4. Using a JTextComponent to capture key presses is the wrong thing to do. Instead, your MyDrawPanel class should implement KeyListener and you can capture your keyPressed events there instead. This saves you from having to create and use the controls field altogether.