HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavaMinor

How to separate logic and GUI

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
logicseparatehowandgui

Problem

I'm writing a program to build and solve a Maze using DFS and backtracking using Java Swing. The code was a mess when I have to put my logic into my JPanel in order to show animation via the call to repaint(). Here are all classes:

Point

public class Point {
    public int mX;
    public int mY;

    public Point(int x, int y) {
        mX = x;
        mY = y;
    }
}


Turtle

```
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.geom.AffineTransform;

public class Turtle {
private int mXPos;
private int mYPos;
private Color mColor;

public Turtle(Color c) {
mXPos = 0;
mYPos = 0;
mColor = c;
}

public void setPosition(int x, int y) {
mXPos = x;
mYPos = y;
}

public int getXPos() {
return mXPos;
}

public int getYPos() {
return mYPos;
}

public void draw(Graphics g) {
int width = 15;
int height = 18;
int heading = 0;
int xPos = mXPos;
int yPos = mYPos;

// cast to 2d object
Graphics2D g2 = (Graphics2D) g;
// save the current transformation
AffineTransform oldTransform = g2.getTransform();

// rotate the turtle and translate to xPos and yPos
g2.rotate(Math.toRadians(heading),xPos,yPos);

// determine the half width and height of the shell
int halfWidth = (int) (width/2); // of shell
int halfHeight = (int) (height/2); // of shell
int quarterWidth = (int) (width/4); // of shell
int thirdHeight = (int) (height/3); // of shell
int thirdWidth = (int) (width/3); // of shell

// draw the body parts (head)
g2.setColor(mColor);
g2.fillOval(xPos - quarterWidth, yPos - halfHeight - (int) (height/3), halfWidth, thirdHeight);
g2.fillOval(xPos - (2 * thirdWidth), yPos - thirdHeight, thirdWidth, thirdHeight);
g2.fillOval(x

Solution

Here's a few tips:

  • When I ran you program at first, I got a blank frame. Make sure frame.setVisible(true) is the last thing you do. Everything else should be set up before that.



-
Really, really, really avoid magic numbers. Code like this:

public void draw(Graphics g) {
    int width = 15;
    int height = 18;
    // etc.
}


Is really hard to maintain. You have a few constants. Just add more.

-
Swing uses swing Timer and SwingWorker rather than threads. For example,

private class MazeSolvingWorker extends SwingWorker {

    // Stores its own copy of the map
    private Boolean[][] map_;

    // Needs to get map from main program
    public MazeSolvingWorker(Boolean[][] map) {
        map_ = map;
    }

    // This function replaces the run method
    @Override
    protected Void doInBackground() throws Exception {
        solveMazeBacktracking(0);
        return null;
    }

    // Need to override when you use publish
    @Override
    protected void process(List chunks) {
        mMap = chunks.get(chunks.size() - 1);
        repaint();
    }

    private void solveMazeBacktracking(int u) {
        mCurrentVertex = u;
        if (u == mGoalVertex || noMoreVertices()) {
            flag = false;
            return;
        }

        mTurtleExplored[u] = true;

        // for each neighbor of u make a move
        List neighbors = getTurtleNeighbors(u);
        for (int i = 0; i < neighbors.size(); ++i) {
            int v = neighbors.get(i);
            if (map_[u][v] == true || map_[v][u] == true) {
                delay(SOLVE_TIME);
                // Thread-safe alternative to calling repaint directly
                publish(map_);
                solveMazeBacktracking(v);
            }
        }
    }

}


Now you can replace this:

item.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        flag = true;
        (new Thread() {
            public void run() {
                solveMazeBacktracking(0);
            }
        }).start();
    }
});


with this:

item.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        flag = true;
        new MazeSolvingWorker(mMap).execute();
    }
});


Replacing buildMazeDfs(int) is up to you.

-
Normally, I would recommend using the entity-system framework to separate logic from data and ui, but you have a very simple program. Factor out a Maze class.

  • This is just nit-picky, but you don't need a static method in MazeBuilder to build the gui. You can move that to Program, where it makes more sense. Or you could get rid of the Program class and put main in MazeBuilder. Just a style preference though. :)

Code Snippets

public void draw(Graphics g) {
    int width = 15;
    int height = 18;
    // etc.
}
private class MazeSolvingWorker extends SwingWorker<Void, Boolean[][]> {

    // Stores its own copy of the map
    private Boolean[][] map_;

    // Needs to get map from main program
    public MazeSolvingWorker(Boolean[][] map) {
        map_ = map;
    }

    // This function replaces the run method
    @Override
    protected Void doInBackground() throws Exception {
        solveMazeBacktracking(0);
        return null;
    }

    // Need to override when you use publish
    @Override
    protected void process(List<Boolean[][]> chunks) {
        mMap = chunks.get(chunks.size() - 1);
        repaint();
    }

    private void solveMazeBacktracking(int u) {
        mCurrentVertex = u;
        if (u == mGoalVertex || noMoreVertices()) {
            flag = false;
            return;
        }

        mTurtleExplored[u] = true;

        // for each neighbor of u make a move
        List<Integer> neighbors = getTurtleNeighbors(u);
        for (int i = 0; i < neighbors.size(); ++i) {
            int v = neighbors.get(i);
            if (map_[u][v] == true || map_[v][u] == true) {
                delay(SOLVE_TIME);
                // Thread-safe alternative to calling repaint directly
                publish(map_);
                solveMazeBacktracking(v);
            }
        }
    }

}
item.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        flag = true;
        (new Thread() {
            public void run() {
                solveMazeBacktracking(0);
            }
        }).start();
    }
});
item.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        flag = true;
        new MazeSolvingWorker(mMap).execute();
    }
});

Context

StackExchange Code Review Q#11383, answer score: 5

Revisions (0)

No revisions yet.