Make this code more Object-Oriented?

I've been told by my friend that my code isn't very OO. 开发者_StackOverflow中文版How would I make this more OO and reusable? I tried making a class called level, that printed the images in level, but noting printed, so that didn't work.

(code dump coming, I'm sorry)

public class Main extends Applet implements Runnable, KeyListener,
java.awt.event.MouseListener {
double x_pos = 300;
double y_pos = 200;
int radius = 20;
int appletsize_x = 640;
int appletsize_y = 440;
double speed = 3;
float speedModifier = 1f;
Image grass;
Image hero;
Image hero45;
Image hero90;
Image hero135;
Image hero180;
Image hero225;
Image hero270;
Image hero315;
Image sprite;
Image tree;
Image path;

private boolean leftPressed;
private boolean rightPressed;
private boolean downPressed;
private boolean upPressed;

private Image dbImage;
private Graphics dbg;

public void init() {
    Dimension dim = getMaximumSize();
    this.setSize(appletsize_x, appletsize_y);
    MediaTracker mt = new MediaTracker(this);
    tree = getImage(getCodeBase(), "tree_64.png");
    grass = getImage(getCodeBase(), "grassTile.png");
    path = getImage(getCodeBase(), "path.png");
    hero = getImage(getCodeBase(), "hero.png");
    hero45 = getImage(getCodeBase(), "hero45.png");
    hero90 = getImage(getCodeBase(), "hero90.png");
    hero135 = getImage(getCodeBase(), "hero135.png");
    hero180 = getImage(getCodeBase(), "hero180.png");
    hero225 = getImage(getCodeBase(), "hero225.png");
    hero270 = getImage(getCodeBase(), "hero270.png");
    hero315 = getImage(getCodeBase(), "hero315.png");
    sprite = getImage(getCodeBase(), "hero.png");
    mt.addImage(hero, 0);
    mt.addImage(path, 0);
    mt.addImage(tree, 0);
    mt.addImage(grass, 0);
    mt.addImage(hero45, 0);
    mt.addImage(hero90, 0);
    mt.addImage(hero135, 0);
    mt.addImage(hero180, 0);
    mt.addImage(hero225, 0);
    mt.addImage(hero270, 0);

    try {
        mt.waitForID(0);
    } catch (InterruptedException ie) {
    }
}

public void start() {

    this.addKeyListener(this);
    this.addMouseListener(this);
    Thread th = new Thread(this);
    th.start();
}

public void stop() {

}

public void destroy() {

}

public void run() {

    // lower ThreadPriority
    this.requestFocusInWindow();
    Thread.currentThread().setPriority(Thread.MIN_PRIORITY);

    while (true) {

        if (downPressed && leftPressed) {
            double y_decr, x_incr;
            y_decr = Math.sqrt(Math.pow(speed, 2)) / 1.5;
            x_incr = Math.sqrt(Math.pow(speed, 2)) / 1.5;
            y_pos += y_decr;
            x_pos -= x_incr;
        } else if (downPressed && rightPressed) {
            y_pos += Math.sqrt(Math.pow(speed, 2)) / 1.5;
            x_pos += Math.sqrt(Math.pow(speed, 2)) / 1.5;
        } else if (upPressed && rightPressed) {
            y_pos -= Math.sqrt(Math.pow(speed, 2)) / 1.5;
            x_pos += Math.sqrt(Math.pow(speed, 2)) / 1.5;
        } else if (upPressed && leftPressed) {
            y_pos -= Math.sqrt(Math.pow(speed, 2)) / 1.5;
            x_pos -= Math.sqrt(Math.pow(speed, 2)) / 1.5;
        } else {
            // Hitting (right)
            if (x_pos > this.getSize().width - radius) {

                // x_speed = -x_speed;
            }
            if (leftPressed == true) {
                x_pos -= speed * speedModifier;
            }

            if (rightPressed == true) {
                x_pos += speed * speedModifier;
            }

            if (upPressed == true) {
                y_pos -= speed * speedModifier;
            }

            if (downPressed == true) {
                y_pos += speed * speedModifier;

            }
        }

        // Hitting right

        if (x_pos > this.getSize().width - 32) {
            x_pos = this.getSize().width - 32;

        } // Hitting left
        if (x_pos < 0) {
            x_pos = 0;
        }
        // } // Hitting top
        if (y_pos < 0) {
            y_pos = 0;

        }

        // Hitting bottom
        if (y_pos > this.getSize().height - 32) {
            y_pos = this.getSize().height - 32;

        }


        repaint();

        try {
            // Stop thread for 1 milliseconds
            Thread.sleep(20);
        } catch (InterruptedException ex) {
            // do nothing
        }

        // set ThreadPriority to maximum value
        Thread.currentThread().setPriority(Thread.MIN_PRIORITY);
    }
}

public void paint(Graphics g) {
    int layout = 0;
    int coll = 0;
    while (coll <= 440) {
        drawRows(layout, grass, g, coll);
        coll = coll + 32;
    }
    drawCollums(0, path, g, 180);
    g.drawImage(tree, 50, 40, this);
    g.drawImage(tree, 300, 20, this);
    g.drawImage(tree, 500, 300, this);
    int x_posI = (int) x_pos;
    int y_posI = (int) y_pos;

    if (downPressed && leftPressed) {
        this.sprite = hero225;
    } else if (downPressed && rightPressed) {
        this.sprite = hero135;
    } else if (upPressed && rightPressed) {
        this.sprite = hero45;
    } else if (upPressed && leftPressed) {
        this.sprite = hero315;
    } else if (leftPressed == true) {
        this.sprite = hero270;
    } else if (rightPressed == true) {
        this.sprite = hero90;
    } else if (upPressed == true) {
        this.sprite = hero;
    } else if (downPressed == true) {
        this.sprite = hero180;
    }

    // this.sprite will contain value set on last "movement"
    g.drawImage(this.sprite, x_posI, y_posI, this);


}



public void update(Graphics g) {

    if (dbImage == null) {
        dbImage = createImage(this.getSize().width, this.getSize().height);
        dbg = dbImage.getGraphics();
    }

    dbg.setColor(getBackground());
    dbg.fillRect(0, 0, this.getSize().width, this.getSize().height);

    dbg.setColor(getForeground());
    paint(dbg);

    g.drawImage(dbImage, 0, 0, this);
}

@Override
public void keyPressed(KeyEvent e) {
    if (e.getKeyCode() == KeyEvent.VK_LEFT) {
        leftPressed = true;

    }
    if (e.getKeyCode() == KeyEvent.VK_RIGHT) {
        rightPressed = true;

    }
    if (e.getKeyCode() == KeyEvent.VK_UP) {
        upPressed = true;
    }
    if (e.getKeyCode() == KeyEvent.VK_DOWN) {
        downPressed = true;
    }

}

@Override
public void keyReleased(KeyEvent e) {

    if (e.getKeyCode() == KeyEvent.VK_LEFT) {
        leftPressed = false;
    }
    if (e.getKeyCode() == KeyEvent.VK_RIGHT) {

        rightPressed = false;
    }
    if (e.getKeyCode() == KeyEvent.VK_UP) {
        upPressed = false;

    }
    if (e.getKeyCode() == KeyEvent.VK_DOWN) {
        downPressed = false;
    }
}

@Override
public void keyTyped(KeyEvent e) {

}

@Override
public void mouseClicked(MouseEvent e) {
    System.out.println("HIT!");
    // TODO Auto-generated method stub

}

@Override
public void mouseEntered(MouseEvent e) {
    // TODO Auto-generated method stub

}

@Override
public void mouseExited(MouseEvent e) {
    // TODO Auto-generated method stub

}

@Override
public void mousePressed(MouseEvent e) {
    // TODO Auto-generated method stub

}

@Override
public void mouseReleased(MouseEvent e) {
    // TODO Auto-generated method stub

}

public void drawRows(int x, Image image, Graphics g, int coll) {
    while (x <= appletsize_x) {
        g.drawImage(image, x, coll, this);
        x += 32;
    }
}

public void drawCollums(int y, Image image, Graphics g, int coll) {
    while (y <= appletsize_x) {
        g.drawImage(image, coll, y, this);
        y += 32;
    }
}
}

You should ask your friend what he or she meant. If I were to guess, I would say that your Applet class is doing a lot of things: it's the animation loop; it's the key handler; it's the mouse handler; it's the UI. Those could be broken out into separate objects that were designed to work together.

Another improvement might be to translate your four ...Pressed flags into an integer from 0 through 7, representing the eight possible directions. For instance:

0 1 2
7   3
6 5 4

(You can assign the direction numbers any way you like.) Then you could use the direction number to index into an array of images. (This has nothing to do with being object-oriented, but it would simplify your code.)

Well for one thing, all those images should be in an array. And they should contain a filename so you can load them in a nice loop. You have over 3 pages of copy&paste code, that's absolutely horrible... What if you want to change the way they get loaded slightly?

Then, Math.sqrt(Math.pow(speed, 2)) jumps out at me. That's the equivalent of Math.abs(speed), and about 1/200x as fast.

This next part is particularly funny too. I won't comment on whether or not it's needed (and it most likely isn't, it's a design flaw on your part), but.. ya..

        // Stop thread for 1 milliseconds
        Thread.sleep(20);

I'd suggest you read books and article about OOP and in this particular case you should take a look at the MVC pattern (MVC article in wikipedia). For a good OO conception the events (keyPressed(KeyEvent e) for example) shouldn't be treated in the same class as the actual "graphics building" class.

Build a new package for your application and group all your data structures inside. Create a class for Your Hero, create classes for trees and grass. See how they can share attributes and create a class hierarchy, maybe with an abstract class.

These are just a few cues, it could be long to give you an Object Oriented course here ;)

But follows this tracks and read.

Also, you should learn how to stop threads elegantly.

And you could use inner classes for your listeners, even use a MouseAdater.

Regards, Stéphane

You might like to study this game that also moves a player around a grid using the keyboard.