Sign in to follow this  
  • entries
    68
  • comments
    177
  • views
    81274

Refactoring with Pong, ep 2

Sign in to follow this  
capn_midnight

233 views

okay, last time on Refactoring with Pong, we demonstrated a poorly designed program, a pong game written in under 30 minutes, with no attention paid to design, naming convention, or documentation.

Now, the engineering process wasn't completely lost. A lot of problems were completely skipped because I used Eclipse. There are a lot of times where Eclipse just won't let you be a stupid programmer, unless you are absolutely ademant about it. Specifically, Eclipse made sure I had all the right classes imported (as well as made sure there were no extra packages imported), as well as made sure each method stub for each interface was at least included in the source file.

However, Eclipse did NOT tell me when I multiplied the y coordinate of the ball by -1 instead of multiplying the y component of the ball's velocity by -1 for bouncing off the side walls. Damn you, you stupid piece of garbage, do what I mean!

Okay, let's discuss the code:

import java.awt.Color;
import java.awt.Cursor;
import java.awt.Font;
import java.awt.Graphics2D;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.awt.event.MouseMotionListener;
import java.awt.image.BufferStrategy;
import java.util.Random;

import javax.swing.JFrame;
import javax.swing.Timer;

/*
* Created on Jul 13, 2005
*
*/


public class Pong extends JFrame implements MouseMotionListener, MouseListener, ActionListener
{
int p1x = 10, p1y = 0, p2x, p2y = 0, bx, by, bdx, bdy, s1 = 0, s2 = 0, state = DEAD, minspd = 2;
static int DEAD = 0, PLAY = 1;
Random r = new Random();
BufferStrategy bs;
Timer t;
Pong()
{
super("Pong in 27 minutes");
this.setBounds(0, 0, 800, 600);
this.addMouseMotionListener(this);
this.addMouseListener(this);
this.setVisible(true);
this.createBufferStrategy(2);
this.setBackground(Color.BLACK);
this.setForeground(Color.WHITE);
this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
bs = this.getBufferStrategy();
t = new Timer(1, this);
p2x = this.getWidth() - 10;
t.start();
}
/**
* @param args
*/

public static void main (String[] args)
{
new Pong();

}
public void mouseDragged (MouseEvent arg0)
{
// TODO Auto-generated method stub

}
public void mouseMoved (MouseEvent arg0)
{
p1y = arg0.getY();
}
public void actionPerformed (ActionEvent arg0)
{
update();
paint();

}
public void paint()
{
Graphics2D g = (Graphics2D) bs.getDrawGraphics();
g.clearRect(0, 0, this.getWidth(), this.getHeight());
g.setColor(Color.WHITE);
g.fillRect(bx-5, by-5, 10, 10);
g.fillRect(p1x-5, p1y-20, 10, 40);
g.fillRect(p2x-5, p2y-20, 10, 40);
g.setFont(new Font("fixedsys", 24, 24));
g.drawString(""+s1, this.getWidth()/3, 60);
g.drawString(""+s2, this.getWidth()*2/3, 60);
g.drawLine(this.getWidth()/2, 0, this.getWidth()/2, this.getHeight());
if(state == DEAD)
{
g.drawString("click a mouse button to start play", this.getWidth() - 500, this.getHeight()/2);
}
bs.show();

}
public void update()
{
if(state == PLAY)
{
bx += bdx;
by += bdy;
if(bx >= this.getWidth()){
s1++;
state = DEAD;
}
else if(bx<= 0){
s2++;
state = DEAD;
}
if(40>= by || by>=this.getHeight())
{
bdy*=-1;
}
checkPaddles();
AIMove();
}

}
public void checkPaddles()
{
if(p1x - 5 <= bx && p1x + 5 >= bx && p1y - 20 <= by && p1y + 20 >= by)
{
int dy = by - p1y;
bdx *= -1;
bdy += dy/10;

}
else if(p2x - 5 <= bx && p2x + 5 >= bx && p2y - 20 <= by && p2y + 20 >= by)
{
int dy = by - p2y;
bdx *= -1;
bdy += dy/10;
}
}
public void AIMove()
{
if(by > p2y) p2y += 7+minspd/2;
if(by < p2y) p2y -= 7+minspd/2;
}
public void mouseClicked (MouseEvent arg0)
{
if(state == DEAD)
{
bx = this.getWidth()/2;
by = this.getHeight()/2;
bdx = r.nextInt(5)+minspd;
bdy = r.nextInt(5)+minspd;
minspd++;
state = PLAY;
}

}
public void mouseEntered (MouseEvent arg0)
{

}
public void mouseExited (MouseEvent arg0)
{
// TODO Auto-generated method stub

}
public void mousePressed (MouseEvent arg0)
{
// TODO Auto-generated method stub

}
public void mouseReleased (MouseEvent arg0)
{
// TODO Auto-generated method stub

}
}


there are so many things wrong with this code, I don't even know where to begin. Probably the biggest thing is that only half the code is there.

"Wait... what?" you say. "I'm looking at the code right now. I see two paddles, a ball, a window to play in, and enough processing to actually get the game running, even have score keeping. What could be missing?"

Tests. This code is not tested. The fact that it even ran the first time was only because Eclipse let's you know when the code is free of syntax errors AS YOU CODE. I wrote code without ever writing a single test first, and that is just a cardinal sin.

Though tests are not actually a refactoring process, they are essential to refactoring. The tests are what tells you "you stupid programmer, you 'fixed' the code and now nothing works". They also tell you exactly what happened after you "fixed" the code.

What else is wrong... bad variable names, lack of any reasonable OOP features, completely monolithic design, duplicated code (maybe I could have got it done in less than 27 minutes if I hadn't duplicated so much code), and a GUI intertwined into the logic code causing a tanlge worse than the cables behind your desk (seriously, how do they get that way? I remember draping them neatly back there, why the tangle?). It's amazing I ever got this thing to run.

Ugh, I've sickened myself. I need to stop for today. Think about these problems and we'll meet again tomorrow.
Sign in to follow this  


0 Comments


Recommended Comments

There are no comments to display.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now