Initialization
As written, your robot lander can land at any X,Y coordinate on your grid, but will always be facing in the positive Y-axis direction. This seems unreasonable. If wind, turbulence can cause a position uncertainty, which requires initializing the robot at a particular X,Y coordinate, it seems reasonable to assume it might also land in any facing.
Robot() {
this(0, 0, 0);
}
Robot(int x, int y) {
this(x, y, 0);
}
Robot(int x, int y, int initial_facing) {
// ...
}
Commands & Instructions
Your instructions are a series of single letter commands, but you can't actually send a single command to the robot. You should separate the individual commands from the series of instructions. Something like:
void turn_left() { ... }
void turn_right() { ... }
void move_forward() { ... }
public void command(char command_letter) {
switch (command_letter) {
case 'L': turn_left(); break;
case 'R': turn_right(); break;
case 'F': move_forward(); break;
default: throw new IllegalArgumentException("Invalid command: "+command_letter);
}
}
public void instructions(String moves) {
for (char command_letter : moves.toCharArray()) {
command(command_letter);
}
}
Where am I?
position()
returns a String
containing the coordinates of the robot. If a control program wants to query the position of the robot, in order to determine what commands should be sent to send it to the desired location, it would need to parse that string back into integer values.
Consider instead returning the actual integer positions, possibly like:
public int[] position() {
return new int[]{ x, y };
}
Alternately, you might want to create a class Position
which can store the x,y location of the robot. Or you could use java.awt.Point
public Point position() {
return new Point(x, y);
}
Perhaps override the toString()
method to return a human friendly description of the robot, including its position. Or maybe a position_as_string()
method.
Which way is the robot facing? Can't directly tell! You currently have to query the position()
, then move("F")
, followed by position()
, and then compare the positions to determine which way the robot is facing! How about adding a facing()
method?
Learn how to write proper unit tests. For example, with JUnit5, you could write:
import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Test;
class RobotTests {
private final Robot robot = new Robot();
@Test
void starts_at_origin() {
assertEquals("0:0", robot.position());
}
@Test
void move_forward_twice() {
robot.move("FF");
assertEquals("0:2", robot.position());
}
@Test
void move_and_turn_right() {
robot.move("FFRF");
assertEquals("1:2", robot.position());
}
@Test
void three_rights_make_a_left() {
robot.move("FFRRRFF");
assertEquals("-2:2", robot.position());
}
@Test
void but_one_left_does_not() {
robot.move("FFLFF");
assertEquals("-2:2", robot.position());
}
}
Notice that each test is run in a brand-new RobotTests
instance, so you don't need to call robot.reset()
between each.
If you run this unit test, you'll find 4 of 5 tests pass, one test fails. I'll leave you to figure out why.
Additional Concerns
currentDirection
taking on the values 0
, 1
, 2
and 3
to represent the 4 cardinal directions is limiting. If later, you want to add in diagonal moves (NW
, SW
, SE
, or NE
), would you use the values 4 and above to represent them? Or would you renumber the original 4 directions to be 0
, 2
, 4
and 6
, and use 1
, 3
, 5
, and 7
for the diagonal directions?
You might be tempted to use an enum
for the direction values, but I think that would be a bad idea. I would use 0
, 90
, 180
, and 270
as direction values. These have physical meaning. If later your robot is allowed a more realistic double x, y;
coordinate system, you could also change to use double currentDirection;
and allow turns of a fraction of a degree. With enum
, you'd lose this possible future flexibility.
As an alternative, you might also consider using a directional vector:
int dx=0, dy=1; // currentDirection = 0°
And move_forward simply becomes:
x += dx;
y += dy;
And a turn right could become:
int old_dx = dx;
dx = dy;
dy = -old_dx;