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

Farm tractor implementation

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

Problem

I have a code which was not refactored at all. I refactored it to some extent, but stuck at a point where I cannot think of anything further.

Tractor.java:

package com.farm;

public class Tractor implements MethodsInterface {

    private int[] position;

    private int[] field;

    private String orientation;

    public Tractor(){
        position  = new int[]{0,0};
        field = new int[]{5,5};
        orientation = "N";
    }

    public void move(String command) {
        if(command=="F"){
            moveForwards();
        }else if(command=="T"){
            turnClockwise();
        }
    }

    private void moveForwards() {
        if(orientation=="N"){
            position = new int[]{position[0], position[1]+1}; 
        } else if(orientation == "E"){ 
            position = new int[]{position[0]+1, position[1]}; 
        } else if(orientation == "S"){ 
            position = new int[]{position[0], position[1]-1}; 
        }else if(orientation == "W"){ 
            position = new int[]{position[0]-1, position[1]}; 
        } 
        if(position[0] > field[0] || position[1] > field[1]) {
            try {
                throw new TractorInDitchException();
            } catch (TractorInDitchException e) {
                e.printStackTrace();
            }
        }

    }

    private void turnClockwise() {
        if(orientation=="N"){
            orientation = "E";
        }else if(orientation == "E"){
            orientation = "S";
        }else if(orientation == "S"){
            orientation = "W";
        }else if(orientation == "W"){
            orientation = "N";
        }
    }

    public int getPositionX() {
        return position[0];
    }

    public int getPositionY() {
        return position[1];
    }

    public String getOrientation() {
        return orientation;
    }
}


TractorInDitchException.java:

package com.farm;

public class TractorInDitchException extends Exception{

}


MethodsInterface.java:

```
package

Solution

-
Instead of magic constants like "N", "S" etc. use an enum which contains named constants and their values are checked on compile time.

public enum Orientation {
  NORTH, SOUTH, WEST, EAST
}


With plain Strings it's too easy to set an invalid value to the orientation field.

-
You can move the turnClockwise method to an enum:

public enum Orientation {
  NORTH {
    @Override
    public Orientation turnClockwise() {
      return EAST;
    }
  },
  SOUTH {
    @Override
    public Orientation turnClockwise() {
      return WEST;
    }
  },
  WEST {
    @Override
    public Orientation turnClockwise() {
      return NORTH;
    }
  },
  EAST {
    @Override
    public Orientation turnClockwise() {
      return SOUTH;
    }
  };

  public abstract Orientation turnClockwise();

}


It increases cohesion and replaces conditional with polymorphism.

(I'd also try to create a Position class with x and y coordinates and move the statements from the moveForwards method to this class like goNorth(), goWest() etc. Then a new Position moveForward(Position current) method in the Orientation enum would call one of the Position.go*() methods.)

-
I'd change this array:

position = new int[] {0, 0};


to two ints:

private int positionX;
private int positionY;


Indexes here (0 and 1) are magic constants.

-
The moveForwards and the the Tractor class looks unfinished. Noone writes the field array.

-
This:

try {
  throw new TractorInDitchException();
} catch (final TractorInDitchException e) {
  e.printStackTrace();
}


could be written as

new TractorInDitchException().printStackTrace();


without the try-catch structure. A logger framework (for example SLFJ and Logback) would be better here.

-
What should happen when the tractor reach the border of the field? (When the index is negative or greater than or equal to the size of the array.)

Code Snippets

public enum Orientation {
  NORTH, SOUTH, WEST, EAST
}
public enum Orientation {
  NORTH {
    @Override
    public Orientation turnClockwise() {
      return EAST;
    }
  },
  SOUTH {
    @Override
    public Orientation turnClockwise() {
      return WEST;
    }
  },
  WEST {
    @Override
    public Orientation turnClockwise() {
      return NORTH;
    }
  },
  EAST {
    @Override
    public Orientation turnClockwise() {
      return SOUTH;
    }
  };

  public abstract Orientation turnClockwise();

}
position = new int[] {0, 0};
private int positionX;
private int positionY;
try {
  throw new TractorInDitchException();
} catch (final TractorInDitchException e) {
  e.printStackTrace();
}

Context

StackExchange Code Review Q#8238, answer score: 8

Revisions (0)

No revisions yet.