5
\$\begingroup\$

I have implemented a simulation which is called "Langton's Ant" with Java. Here is a short summary of basic rules:

  • A ant is placed in a 2D matrix and looks to north, west, east or south. First, all cells are colored white.

  • If the ant is on a white cell, the cell is colored black and the ant turns 90 degrees right and moves to the next cell in this direction.

  • If the ant is on a black cell, the cell is colored white and the ant turns 90 degrees left and moves to the next cell in this direction.

A white cell is represented by a 0 in array and a black cell is represented by a 1 in array.

Ant:

public class Ant {

    private Direction direction;
    private int positionX, positionY;   
    private World world = new World();
    private final int steps = 10000;

    public Ant(int positionX, int positionY, Direction direction) {
        this.positionX = positionX;
        this.positionY = positionY;
        this.direction = direction;     
    }

    public int getSteps() {
        return steps;
    }

    public int getPositionX() {
        return this.positionX;
    }

    public int getPositionY() {
        return this.positionY;
    }

    public void setDirection(Direction direction) {
    this.direction = direction;
    }

    public Direction getDirection() {
        return this.direction;
    }

    public boolean inWorld(Ant a) {     
        if(a.getPositionX()<=world.getWorldSize()-2 && a.getPositionY()<=world.getWorldSize()-2
                && a.getPositionX()>=1 && a.getPositionY()>=1) {        
            return true;
        }       
        return false;
    }

    public boolean nextStep(int w[][],Ant a) {

    if(inWorld(a) == true) {

    if(w[a.getPositionX()][a.getPositionY()]==0 && a.getDirection()==Direction.North) {
        a.setDirection(Direction.East); 
        w[a.getPositionX()][a.getPositionY()]=1; 
        a.positionY--;
     }

    if(w[a.getPositionX()][a.getPositionY()]==0 && a.getDirection()==Direction.East) {
        a.setDirection(Direction.South);
        w[a.getPositionX()][a.getPositionY()]=1;
        a.positionX++; 
        }

    if(w[a.getPositionX()][a.getPositionY()]==0 && a.getDirection()==Direction.South) {
        a.setDirection(Direction.West);
        w[a.getPositionX()][a.getPositionY()]=1;
        a.positionY++;  
        }

    if(w[a.getPositionX()][a.getPositionY()]==0 && a.getDirection()==Direction.West) {
        a.setDirection(Direction.North); 
        w[a.getPositionX()][a.getPositionY()]=1; 
        a.positionX--; 
        }

    if(w[a.getPositionX()][a.getPositionY()]==1 && a.getDirection()==Direction.North) {  
        a.setDirection(Direction.West);
        w[a.getPositionX()][a.getPositionY()]=0;  
        a.positionY++;  
        }

    if(w[a.getPositionX()][a.getPositionY()]==1 && a.getDirection()==Direction.East) {
        a.setDirection(Direction.North);
        w[a.getPositionX()][a.getPositionY()]=0; 
        a.positionX--; 
        }

    if(w[a.getPositionX()][a.getPositionY()]==1 && a.getDirection()==Direction.South) {
        a.setDirection(Direction.East); 
        w[a.getPositionX()][a.getPositionY()]=0;
        a.positionY--; 
        }

    if(w[a.getPositionX()][a.getPositionY()]==1 && a.getDirection()==Direction.West) { 
         a.setDirection(Direction.South);
         w[a.getPositionX()][a.getPositionY()]=0; 
         a.positionX++; 
        }

      return true;
      } 
      return false;
     }
}

Direction:

public enum Direction {
    North, East, South, West
}

World:

public class World {

    private final int worldSize =400;
    private int world[][] = new int[worldSize][worldSize];

    public int[][] getWorld() {
        return world;
    }


    public int getWorldSize() {
        return worldSize;
    }
}

AntFrame:

public class AntFrame {

    private Ant a = new Ant(50, 50, Direction.South);   
    private World w = new World();
    private int world[][] = w.getWorld();

    private int steps = a.getSteps();
    private Timer timer = new Timer();
    private JFrame f = new JFrame();
    private int worldSize = w.getWorldSize();
    private AntPanel antPanel = new AntPanel();

    public AntFrame() {
        f.setSize(600, 600);
        f.setTitle("Langtons Ant");
        f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        f.setLayout(new FlowLayout());
        f.add(antPanel);
        f.setVisible(true);     
        world[50][50] =1;
        a.setDirection(Direction.South);
        timer.schedule(new SimulationThread(), 400, 5);

    }

    class SimulationThread extends TimerTask {

        private int tmp =0;

        @Override
        public void run() {


                if(a.nextStep(world,a)==true) {

                    a.nextStep(world, a);
                    antPanel.repaint();
                   tmp = tmp+1;
                }
                else {
                    timer.cancel();
                }                                       

        if(tmp==steps) {
            timer.cancel();
        }               
      } 
    }
    class AntPanel extends JPanel {

        public AntPanel() {
            setPreferredSize(new Dimension(600, 600));
            setBackground(Color.WHITE);
        }

        @Override
        public void paintComponent(Graphics g) {
            super.paintComponent(g);

            for (int i = 0; i < world.length; i++) {
                for (int j = 0; j < world.length; j++) {

                    if (world[i][j] == 1) {
                        g.setColor(Color.BLACK);
                        g.fillRect(i * 3, j * 3, 3, 3);

                    }
                }
            }
        }
     }

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

It would be really nice if someone could give me feedback.

\$\endgroup\$
  • 1
    \$\begingroup\$ Your first condition in Ant.nextStep(), if(w[a.getPositionX()][a.getPositionY()]==0 && a.getDirection()==Direction.North) a.setDirection(Direction.West);, says that if the ant is on a white cell, it turns left. This is in contradiction to your specification. \$\endgroup\$ – 200_success Jun 13 '18 at 20:05
  • 1
    \$\begingroup\$ I have edited the question. \$\endgroup\$ – Marten Jun 13 '18 at 22:05
  • 1
    \$\begingroup\$ I know that you were asked to update your post so the description and code are consistent, and while you were working on that you received an answer. Be cautious with edits after an answer is received, since modifying the code could invalidate that answer (in some cases we refer people to what you may and may not do after receiving answers but I don't think that is a major concern here). \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jun 13 '18 at 22:46
8
\$\begingroup\$

Bugs

Your simulation is buggy — and not just because an ant is involved!

It looks like you intend for each call of AntFrame.SimulationThread.run() to execute one step of the ant. However, each call actually results in multiple steps, for two reasons:

  • run() calls nextStep(…) twice. Furthermore, on the second call, it disregards the bounds check.
  • Within nextStep(…), the eight cases are not mutually exclusive. Rather, it's often the case that after a step is made, execution falls through to another if block.

Note that the inWorld(…) bounds check is only done once, at the start of each call to run(). Therefore, it is possible to step out of bounds and cause a crash. (You have masked the effects of this bug by making the inWorld(…) bounds comparisons extra conservative.) Furthermore, since the step counter is only occasionally incremented, the Ant.steps limit of 10000 is incorrectly enforced. Also, the repainting of the display is requested less often than appropriate.

The way you have oriented your axes, "north" is towards the left of the screen.

The size of the JFrame and JPanel, 600×600, is insufficient to show a world of 400×400 cells. Ideally, you should calculate the pixel dimensions by multiplying the world size by the number of pixels per cell. Furthermore, instead of explicitly and redundantly calling f.setSize(…, …), you should just set the JPanel's preferred size, then call f.pack() to have the frame calculate its own appropriate size.

Swing methods should be called on the event dispatch thread.

Object-oriented design

It's confusing that AntFrame is not a JFrame, as its name would suggest. A more appropriate name would be AntSim.

In object-oriented programming, objects are nouns, and methods are verbs. Instantiating an object should just bring a data structure, properly initialized, into existence; it should not also trigger events to happen. Therefore, your AntFrame() constructor should not start the simulation thread.

How many Worlds should there be? You instantiated two worlds. One of them is a member variable of AntFrame. There is also another one inside Ant — but that one is used for nothing but to get the world size for the inWorld(…) bounds check.

World is an underdeveloped class: as it is, it is just a glorified reference to a two-dimensional array. You can't do anything with it except through the matrix returned by getWorld(). That's bad design, because once you have returned the matrix, the caller has to read from and write to the matrix directly, breaking encapsulation. Also, if you have to fetch the matrix, then getWorldSize() is not that useful, since the dimensions of the matrix can be determined using .length anyway.

Direction is an underdeveloped enum. For each direction, you should be able to easily get the directions that are 90° to its left and right. Furthermore, each direction should declare how a step in that direction should change the coordinates, in terms of (Δx, Δy). With those improvements, you should be able to reduce the number of conditions in Ant.nextStep() by a factor of 4.

It makes no sense for Ant.inWorld() and Ant.nextStep() to accept an Ant parameter. Those are already methods of the Ant class; the ant that they act on should just be this.

I would expect Ant.step to keep track of the number of steps that the ant has taken. Your Ant.step is just a constant, and I would consider the step limit to be a property of the simulation rather than of the ant.

The AntFrame() constructor redundantly initializes the ant's direction to south twice.

Suggested solution

Ant.java

public class Ant {
    public static enum Direction {
        NORTH(0, -1),
        EAST(+1, 0),
        SOUTH(0, +1),
        WEST(-1, 0);

        public final int deltaX, deltaY;

        private Direction(int deltaX, int deltaY) {
            this.deltaX = deltaX;
            this.deltaY = deltaY;
        }

        /**
         * Returns the <code>Direction</code> that is 90 degrees to the left.
         */
        public Direction left() {
            return Direction.values()[(this.ordinal() + 3) % 4];
        }

        /**
         * Returns the <code>Direction</code> that is 90 degrees to the right.
         */
        public Direction right() {
            return Direction.values()[(this.ordinal() + 1) % 4];
        }
    }

    private int x, y, steps;
    private Direction direction;

    public Ant(int x, int y, Direction direction) {
        this.x = x;
        this.y = y;
        this.direction = direction;
    }

    public int getSteps() {
        return this.steps;
    }

    public int getX() {
        return this.x;
    }

    public int getY() {
        return this.y;
    }

    public Direction getDirection() {
        return this.direction;
    }

    public boolean isInWorld(World w) {
        return 0 <= this.getX() && this.getX() < w.getSize() &&
               0 <= this.getY() && this.getY() < w.getSize();
    }

    public void step(World w) {
        World.Color c = w.getCellColor(this.getX(), this.getY());
        this.direction = (World.Color.WHITE == c) ? this.direction.right()
                                                  : this.direction.left();
        w.setCellColor(this.getX(), this.getY(), c.inverse());
        this.x += this.direction.deltaX;
        this.y += this.direction.deltaY;
        this.steps++;
    }

    public String toString() {
        return String.format(
            "Ant(%4d, %4d, %s)", this.getX(), this.getY(), this.getDirection()
        );
    }
}

World.java

public class World {
    public static enum Color {
        WHITE, BLACK;

        public Color inverse() {
            return WHITE.equals(this) ? BLACK : WHITE;
        }
    }

    private Color[][] cells;

    public World(int size) {
        this.cells = new Color[size][size];
        for (int x = 0; x < size; x++) {
            for (int y = 0; y < size; y++) {
                this.cells[x][y] = Color.WHITE;
            }
        }
    }

    public int getSize() {
        return this.cells.length;
    }

    public Color getCellColor(int x, int y) {
        return this.cells[x][y];
    }

    public void setCellColor(int x, int y, Color c) {
        this.cells[x][y] = c;
    }
}

AntSim.java

import java.awt.Color;
import java.awt.Dimension;
import java.awt.FlowLayout;
import java.awt.Graphics;
import java.util.Timer;
import java.util.TimerTask;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;

public class AntSim {
    private JFrame frame;
    private JPanel antPanel;
    private World world;
    private Ant ant;

    public class AntPanel extends JPanel {
        private final int dotSize;

        public AntPanel(int dotSize) {
            int pixels = dotSize * world.getSize();
            this.dotSize = dotSize;
            this.setPreferredSize(new Dimension(pixels, pixels));
            this.setBackground(Color.WHITE);
        }

        @Override
        public void paintComponent(Graphics g) {
            super.paintComponent(g);
            for (int x = 0; x < world.getSize(); x++) {
                for (int y = 0; y < world.getSize(); y++) {
                    if (world.getCellColor(x, y) == World.Color.BLACK) {
                        g.setColor(Color.BLACK);
                        g.fillRect(dotSize * x, dotSize * y, dotSize, dotSize);
                    }
                }
            }
        }
    }

    public AntSim() {
        this.world = new World(100);
        this.world.setCellColor(50, 50, World.Color.BLACK);
        this.ant = new Ant(50, 50, Ant.Direction.SOUTH);

        this.frame = new JFrame("Langton's Ant");
        this.frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.frame.setLayout(new FlowLayout());
        this.frame.add(this.antPanel = new AntPanel(3));
        this.frame.pack();
        this.frame.setVisible(true);
    }

    public void run(int maxSteps, long delay, long period) {
        Timer timer = new Timer();
        timer.schedule(new TimerTask() {
            @Override
            public void run() {
                if (!(ant.getSteps() < maxSteps && ant.isInWorld(world))) {
                    timer.cancel();
                } else {
                    SwingUtilities.invokeLater(() -> { antPanel.repaint(); });
                    ant.step(world);
                }
            }
        }, delay, period);
    }

    public static void main(String... args) {
        SwingUtilities.invokeLater(() -> {
            new AntSim().run(15000, 400, 5);
        });
    }
}
\$\endgroup\$
3
\$\begingroup\$
  1. if(a.getPositionX()<=world.getWorldSize()-2 && a.getPositionY()<=world.getWorldSize()-2 && a.getPositionX()>=1 && a.getPositionY()>=1) { Magical numbers, in this case "2", should be avoided, because by looking at this number you have no idea what it expresses. In my personal experience it's better to compare with zero, not one: a.getPositionX() > 0
  2. if(inWorld(a) == true) { Redundant comparison with true. Could be just: if(inWorld(a)). I prefer to return early from the method to decrease the level of blocks nesting: if(!inWorld(a)) { return false; }
  3. public boolean nextStep(int w[][],Ant a) The body of this method could be represented in declarative style as a table. We basically set new direction of ant, color of cell and new position of ant depending on current ant direction and color of cell. We could create enum for colors (black and white) and class to represent position delta. So, it could look like this (in pseudocode):

    Color.White, Direction.West -> Direction.North, Color.Black, Delta(1, 0)
    Color.Black, Direction.North-> Direction.West, Color.White, Delta(0, -1)
    ...
    
  4. private final int worldSize =400; Seems like this is a property of a class, not a constant, because practically you can create a world of any size and can have few worlds with different sizes.
  5. public boolean inWorld(Ant a) { This method should have World as parameter not Ant, because you already have ant's data inside current object. Or you could move this method to World class.
  6. You should encapsulate inner structure of class World. This class could store its data as array, list or any other suitable type, but you expose this information (method World.getWorld()), so it would be difficult in the future to change its inner workings. Also, you should make this class more abstract to have methods like: World.changeColor(Color.Black, 50, 50) in order to decrease complexity of changing World.
  7. if(a.nextStep(world,a)==true) {Redundant comparison with true
  8. public void paintComponent(Graphics g) {. In my opinion observer pattern is best suited for this type of relation between model (World) and view(Graphics). So you can subscribe view for changes in a model (World) and make any visual effects in a handler. Then you can update only the model and happily forget about view.
\$\endgroup\$
3
\$\begingroup\$

There is a working example of the Langton's ant on github:

https://github.com/douma/langtons-ant-java

Use value objects to separate concerns and to make the code more readable

This implementation uses value objects for the ant's degree rotation and the position. Using value objects will improve the readability of your code.

Instead of writing

public Ant(int positionX, int positionY, Direction direction) {
    this.positionX = positionX;
    this.positionY = positionY;
    this.direction = direction;     
}

You could write:

public Ant(Position position, Direction direction) {
    this.position = position;
    this.direction = direction;     
}

Where Position is simply an immutable value object:

public class Position
{
    int x, y;

    Position(int x, int y)
    {
        this.x = x;
        this.y = y;
    }

    public Position left()
    {
        return new Position(this.x - 1, this.y);
    }

    public Position right()
    {
        return new Position(this.x + 1, this.y);
    }

    public Position up()
    {
        return new Position(this.x, this.y + 1);
    }

    public Position down()
    {
        return new Position(this.x, this.y - 1);
    }

    public int x()
    {
        return this.x;
    }

    public int y()
    {
        return this.y;
    }

    public String toString()
    {
        return "["+this.x+","+this.y+"]";
    }
}

My advice is to also write some tests if you have not done so. Creating smaller classes makes your implementation easier to test and testing is really beneficial for this specific logic.

Code is hard to read, encapsulate logic and use clear names

By encapsulating the logic more into separate classes, your code becomes more readable. Code like this is hard to read:

public void step(World w) {
    World.Color c = w.getCellColor(this.getX(), this.getY());
    this.direction = (World.Color.WHITE == c) ? this.direction.right()
                                              : this.direction.left();
    w.setCellColor(this.getX(), this.getY(), c.inverse());
    this.x += this.direction.deltaX;
    this.y += this.direction.deltaY;
    this.steps++;
}

Instead, look at the code below from the example it is almost the same as the business rules written in words:

public void moveAnt() throws Exception
{
    for(int x = 0; x < this.length; x++) {
        Position position = this.ant.position();
        if(this.isMarked(position)) {
            this.ant.forwardLeft();
            this.unmark(position);
        } else {
            this.ant.forwardRight();
            this.mark(position);
        }
    }
}

Business rules:

  • At a white square, turn 90° right, flip the color of the square, move forward one unit

  • At a black square, turn 90° left, flip the color of the square, move forward one unit

Avoid using rendering logic in the code.

Completely move this to the JFrame view implementation. So code like this, can be easily moved to the view layer. Instead keep a list of marked Position objects and pass them to the view.

World.Color c = w.getCellColor(this.getX(), this.getY());

This will make your code view independent.

\$\endgroup\$

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.