patternjavaMinor
Farm tractor implementation
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:
TractorInDitchException.java:
MethodsInterface.java:
```
package
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
With plain
-
You can move the
It increases cohesion and replaces conditional with polymorphism.
(I'd also try to create a
-
I'd change this array:
to two ints:
Indexes here (
-
The
-
This:
could be written as
without the
-
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.)
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.