This is my first OOP coding project and I would appreciate any constructive criticism as to how I could have used the benefits of Java and OOP to better effect. How would you have implemented this differently? I have been learning Java for less than a week but I hope to go on to make Android apps. See here for this applet working and also source code: http://fleawhale.net/java/SlidePuzz2/


import java.applet.*;
import java.awt.event.*; 
import java.awt.*;
import java.awt.image.*;
import java.io.*;
import java.net.*;
import javax.imageio.*;
import java.util.*;
import javax.swing.Timer;

public class SlidePuzz2 extends Applet implements MouseMotionListener, MouseListener {
    private static final long serialVersionUID = 1L;
    static Integer animal;
    static Integer grid;
    static int piece;
    static Board board;
    static Element playButton;
    static Element loadingText;
    static boolean startGame;

    Timer aniTimer;

    static String[] imgUrls = { "7028/6814220663_4813a81531",
                "7162/6814220921_acb3aa92ee", "7029/6814221019_41323ace8b" };
    static String imgUrlFlickr = "http://farm8.staticflickr.com/";

    public void init() {
        startGame = false;
        setSize(420, 420); 

        new ElemText(50, 50, "Select animal:");
        new ElemOption(65, 80, "mouse", "animal");
        new ElemOption(65, 110, "cat", "animal");
        new ElemOption(65, 140, "dog", "animal");

        new ElemText(250, 50, "Select grid:");
        new ElemOption(265, 80, "3x3", "grid");
        new ElemOption(265, 110, "4x4", "grid");
        new ElemOption(265, 140, "5x5", "grid");
        new ElemOption(265, 170, "6x6", "grid");
        new ElemOption(265, 200, "7x7", "grid");

        playButton = new ElemOption(40, 280, "Play Game >>", "play", 36, false);

    public void game() {
        startGame = false;

        animal = (Integer)Element.options.get("animal");
        grid = (Integer)Element.options.get("grid");
        grid = grid.intValue()+3;
        piece = 420/grid;
        board = new Board(grid);


        Image img = null;
        BufferedImage img_cr;
        URL url = null;
        try {url = new URL(imgUrlFlickr+imgUrls[animal]+"_o.jpg");}
        catch (MalformedURLException e1) {}
        try {img = ImageIO.read(url);}
        catch (IOException e) {}

        PieceLoc pl = new PieceLoc();
        ArrayList pls = new ArrayList();
        for(int x=0; x<grid; x++) {
            for(int y=0; y<grid; y++) {
                pl = new PieceLoc();
                pl.x = x;
                pl.y = y;

        Random r = new Random();        
        for(int x=0; x<grid; x++) {
            for(int y=0; y<grid; y++) {
              if (x==grid-1 && y==grid-1) break;
              img_cr = ((BufferedImage)img).getSubimage(x*piece, y*piece, piece, piece);
              pl = (PieceLoc)pls.remove(r.nextInt(pls.size()));
              //pl.x=x; pl.y=y; // uncomment to cheat the game
              board.add(new ElemImg(pl.x*piece, pl.y*piece, img_cr), pl);


    public class AniTimer implements ActionListener {
        public Element animating;
        private PieceLoc blank;
        private int orig_x;
        private int orig_y;
        private long timeStart;
        private int delta;

        public AniTimer(Element e, PieceLoc pl) {
            animating = e;
            blank = pl;
            orig_x = animating.x;
            orig_y = animating.y;
            timeStart = System.currentTimeMillis();
        public void actionPerformed(ActionEvent evt) {
            int dx = (blank.x*piece-orig_x);
            int dy = (blank.y*piece-orig_y);
            int t = 200;
            delta = (int)(System.currentTimeMillis()-timeStart);
            if (delta>t) delta=t;
            animating.x = orig_x + dx*delta/t;
            animating.y = orig_y + dy*delta/t;
            if (delta==t) {

    public void paint(Graphics g) {
        if(startGame) game();

    public void update(Graphics g) {

    public void mouseMoved(MouseEvent evt) {
        int x = evt.getX();
        int y = evt.getY();
        if(Element.isClickable(x, y)) setCursor(Cursor.getPredefinedCursor(Cursor.HAND_CURSOR));
        else setCursor(Cursor.getPredefinedCursor(Cursor.DEFAULT_CURSOR));

    public void mousePressed(MouseEvent evt) {
        int x = evt.getX();
        int y = evt.getY();
        Element e = Element.clicked(x, y);
        if(e==null) return;
        if( e.type=="img") {
            aniTimer = new Timer(30, new AniTimer(e, board.getBlankPl()));
        } else if (e.group!=null && e.group=="play") {
            loadingText = new ElemText(70, 255, "Loading game, please wait...");
            startGame = true;

    public void destroy() {

    public void mouseDragged(MouseEvent evt) {}
    public void mouseExited(MouseEvent evt) {}
    public void mouseEntered(MouseEvent evt) {}
    public void mouseClicked(MouseEvent evt) {}
    public void mouseReleased(MouseEvent evt) {}

class PieceLoc {
    int x;
    int y;


import java.awt.*;
import java.awt.image.BufferedImage;
import java.util.*;

public class Element {
    public int x;
    public int y;
    private int h;
    private int w;
    private int minX;
    private int minY;
    private int maxX;
    private int maxY;
    public int mouseBleed;
    private Image img;
    public Image imgAlt;
    public boolean visible;
    public boolean clickable;
    private boolean useAlt;
    public String type;
    public String group;
    public Integer value;

    public static HashMap options = new HashMap();
    private static ArrayList elems = new ArrayList();
    private static ArrayList clickables = new ArrayList();
    public static int imgCount = 0;

    public Element(int x, int y, String type, boolean visible, boolean clickable) {
        this.x = x;
        this.y = y;
        this.type = type;
        this.visible = visible;
        this.clickable = clickable;
        this.useAlt = false;

    static Image draw () {
        Image di;
        BufferedImage buff = new BufferedImage(420, 420, BufferedImage.TYPE_INT_RGB);
        Graphics2D g = buff.createGraphics();

        g.fillRect(0, 0, 420, 420);

        for (int i=0;i<elems.size();i++) {
            Element e = (Element)elems.get(i);
            if (e.useAlt) di = e.imgAlt;
            else di = e.img;
            if(e.visible) g.drawImage(di, e.x, e.y, null);

        return buff;

    private Dimension getTextDim(String s, Font f) {
        BufferedImage buff = new BufferedImage(420, 420, BufferedImage.TYPE_INT_RGB);
        Graphics2D g = buff.createGraphics();

        FontMetrics metrics = g.getFontMetrics();
        int w = metrics.stringWidth(s);
        int h = metrics.getHeight();
        return new Dimension(w, h);

    public Image drawText(String s, Font f, boolean invert) {
        Dimension dim = getTextDim(s, f);
        int w = (int)dim.getWidth();
        int h = (int)dim.getHeight();
        int x = f.getSize();

        BufferedImage buff = new BufferedImage(w+x*2, h, BufferedImage.TYPE_INT_RGB);
        Graphics2D g = buff.createGraphics();

        g.fillRect(0, 0, w+x*2, h);
        if (invert) g.setColor(Color.BLACK); 
        else g.setColor(Color.WHITE);
        g.fillRect(x, 0, w, h);
        g.fillOval(h/4, -1, h, h+1);
        g.fillOval(w+h/4, -1, h, h+1);
        if (invert) g.setColor(Color.WHITE); 
        else g.setColor(Color.BLACK);
        g.drawString(s, x, x);

        return buff;

    public Image drawBlank(int w, int h) {
        BufferedImage buff = new BufferedImage(w, h, BufferedImage.TYPE_INT_RGB);
        Graphics2D g = buff.createGraphics();

        g.fillRect(0, 0, w, h);

        return buff;

    public void setImg(Image img) {
        this.img = img;
        this.w = img.getWidth(null);
        this.h = img.getHeight(null);

    public void updateCA() {
        this.minX = this.x-this.mouseBleed;
        this.minY = this.y-this.mouseBleed;
        this.maxX = this.x+this.w+this.mouseBleed;
        this.maxY = this.y+this.h+this.mouseBleed;

    static boolean isClickable(int x, int y) {
        for (int i=0;i<elems.size();i++) {
            Element e = (Element) elems.get(i);
            if(!e.clickable || !e.visible) continue;
            if(x>e.minX && x<e.maxX && y>e.minY && y<e.maxY) {
                if(e.group!=null) changeSelected(e);
                return true;
        return false;

    static Element clicked(int x, int y) {
        int a;
        String groupClicked="";
        for (int i=0;i<elems.size();i++) {
            Element e = (Element)elems.get(i);
            if(!e.clickable || !e.visible) continue;
            else if(x>e.minX && x<e.maxX && y>e.minY && y<e.maxY) {
                if(e.group=="restart") {
                    SlidePuzz2.loadingText.visible = false;
                } else if(e.group!=null) {
                    options.put(e.group, new Integer(e.value));
                    if(!SlidePuzz2.playButton.visible && options.size()>4) SlidePuzz2.playButton.visible=true;
                return e;
        return null;

    static void changeSelected() {
        for (int i=0;i<elems.size();i++) {
            Element e = (Element)elems.get(i);
            if(e.group==null) continue;
            if(!options.containsKey(e.group)) e.useAlt=false;
            else {
                if(options.get(e.group).equals(e.value)) e.useAlt=true;
                else e.useAlt=false;

    static void changeSelected(Element elem) {
        String g = elem.group;
        int v = elem.value;
        for (int i=0;i<elems.size();i++) {
            Element e = (Element)elems.get(i);
            if(e.group==g && e.value!=v) e.useAlt=false;

    static void setVisibility(boolean b) {
        for (int i=0;i<elems.size();i++) {
            Element e = (Element)elems.get(i);

    static void makeUnclickable() {
        for (int i=0;i<elems.size();i++) {
            Element e = (Element)elems.get(i);
            if(e.clickable && e.visible) e.clickable=false;

    static void destroyVisible() {
        for (int i=0;i<elems.size();i++) {
            Element e = (Element) elems.get(i);
            if(e.visible) {

class ElemText extends Element {
    public ElemText(int x, int y, String s) {
        super(x, y, "text", true, false);
        Font f = new Font("Verdana", Font.BOLD,  14);
        setImg(drawText(s, f, false));

class ElemImg extends Element {
    int index;
    public ElemImg(int x, int y, Image img) {
        super(x, y, "img", true, false);
        index = imgCount;

class ElemBlank extends Element {
    int w;
    int h;
    public ElemBlank(int x, int y, int w, int h) {
        super(x, y, "blank", true, false);
        setImg(drawBlank(w, h));

class ElemOption extends Element {
    public ElemOption(int x, int y, String s, String g, int size, boolean visible) {
        super(x, y, "option", visible, true);
        init(x, y, s, g, size, visible);

    public ElemOption(int x, int y, String s, String g) {
        super(x, y, "option", true, true);
        int size = 12;
        boolean visible = true;
        init(x, y, s, g, size, visible);

    public void init(int x, int y, String s, String g, int size, boolean visible) {
        Integer v;
        if(options.containsKey(g+"_count")) v = (Integer)options.get(g+"_count");
        else v = 0;

        Font f = new Font("Verdana", Font.PLAIN, size);
        mouseBleed = size;
        setImg(drawText(s, f, false));
        imgAlt = drawText(s, f, true);
        clickable = true;
        group = g;
        value = new Integer(v);
        this.visible = visible;

        options.put(g+"_count", new Integer(v));


import java.util.*;

class Board {
    private static int grid;
    private static int piece;

    ArrayList locations = new ArrayList();

    public Board(int grid) {
        this.grid = grid;
        this.piece = 420/grid;
        for(int i=0; i<grid*grid; i++) locations.add(new Integer(0));

    public void add(Element e, PieceLoc pl) {
        locations.set(pl2loc(pl), e);

    public void makeClickables() {
        PieceLoc blankPiece = loc2pl(locations.indexOf(new Integer(0)));
        addClick(blankPiece, -1, 0);
        addClick(blankPiece, 1, 0);
        addClick(blankPiece, 0, -1);
        addClick(blankPiece, 0, 1);

    private void addClick(PieceLoc blankPiece, int dx, int dy) {
        PieceLoc pl = new PieceLoc();
        pl.x = blankPiece.x+dx;
        pl.y = blankPiece.y+dy;
        if (pl.x>=0 && pl.y>=0 && pl.x<grid && pl.y<grid) {

    private PieceLoc loc2pl(int loc) {
        PieceLoc pl = new PieceLoc();
        pl.y = loc % grid;
        pl.x = (loc-pl.y)/grid;
        return pl;

    private int pl2loc(PieceLoc pl) {
        int loc = pl.x*grid+pl.y;
        return loc;

    public PieceLoc getBlankPl() {
        return loc2pl(locations.indexOf(new Integer(0)));

    public void clicked(Element elem) {
        int blank = locations.indexOf(new Integer(0));
        int clicked = locations.indexOf(elem);

    public void checkCompleted() {
        boolean completed = false;
        if(locations.indexOf(new Integer(0))==locations.size()-1) {
            completed = true;
            for(int i=0; i<locations.size()-1; i++) {
                ElemImg e = (ElemImg)locations.get(i);
                if(e.index!=i) completed=false;
        if(completed) {
            new ElemBlank(280, 280, 140, 140);
            new ElemText(292, 290, "Well done,");
            new ElemText(295, 315, "you have");
            new ElemText(292, 340, "completed");
            new ElemText(290, 365, "the puzzle!");
            new ElemOption(300, 390, "Play Again", "restart");

        } else makeClickables();

In SlidePuzzle2.init(), you create a bunch of elements without storing their reference. I understand that the references are stored statically in Element, but it looks very unusual and it should be avoided. You should avoid static members or methods as much as possible in OO programming.

The class Element does way too many things. The main problem is that you did not use the MVC (model-view-controller) pattern. You should define everything abstractly first (model), without thinking in the slightest about showing those things. Once you have completed the model, you can start doing the interface (view). There are two advantages: 1) your code is much cleaner and 2) if you want to port your program to Android later, you don't touch the model, but just rewrite the interface.

P.S.: the controller handles the user input, which changes the model and ultimately the view. You don't have to have classes Model, View, Controller, but each class should belong to the model or the view. Note that in Swing the view and controller are pretty much fused into a view-controller since the Swing components carry both roles.

| improve this answer | |
  • \$\begingroup\$ Thank you, this is truly helpful. For my next project I shall use MVC as my mantra. So suppose I were creating a multi-level maze game. Could examples of Model classes be Maze to represent the current maze, State to represent the current position of items within the maze, and Movement to represent the direction items within the maze are moving? But all of these should be completely independent of how its all represented on the screen, this are all done with the View classes which take look at the properties of the Model objects to do their thing. \$\endgroup\$ – Jimmy_Bob Feb 12 '12 at 15:54
  • \$\begingroup\$ Then the control objects simply listen for any mouse or key activity and call the Models' methods appropriately? \$\endgroup\$ – Jimmy_Bob Feb 12 '12 at 15:55
  • 1
    \$\begingroup\$ Yes, and as I said, in Swing, the controllers are also the view objects -- the JComponents such as JButton. I guess it's the same in Android. But if you were to do a console (terminal) version of your game, than you would read in the move from the command line (the control), which would change the board state (the model) and then you would print some lines to represent the current board (the view). No matter which view/controller, you would always use the same code for the board which can receive a move and change the board state accordingly. The controller updates the view after a move. \$\endgroup\$ – toto2 Feb 12 '12 at 22:13

After a quick look:

  • I would use anonymous inner classes for the listeners. One advantage is that you can inherit from Adapters
  • in Swing, usually you should overwrite paintComponent, not paint
  • ArrayList locations should be generic
  • some methods like game are too long.
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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