Mars Rover Simulator

Question

Problem Statement

Consider a rover and a plateau of size nxn. The rover takes three type of instructions L,R and M. 'L' and 'R' rotate the rover in the left and right direction. 'M' moves the rover one step forward in the direction it faces.

Input: Plateau size, Initial direction, Initial position and Instructions. Output: Final position and direction the rover faces.

Eg. Input: Plateau Size :5 Initial Direction : N Initial Position : 0,0 Instructions: RMML

Output: 2,0 N.

I have written a program that does this. I would like a review on it. Also I have used a grid coordinate system to refer the plateau here. Is there a better way to do it ??

MarsRover.java

/**
 * Created by Leo on 18-07-2015.
   Completed on 23-07-2015
 */

import java.util.Scanner;

public class MarsRover {

    private String inst_set;
    private Plateau p;
    private Cell currentLocation;

    public enum Direction{N,S,E,W}

    Direction dir;

    public MarsRover(Plateau p1, int x, int y, Direction dir, String instSet) {

        p=p1;
        inst_set=instSet;
        this.dir=dir;
        currentLocation=new Cell(x, y);

    }

    public MarsRover(){}

    private void executeInst() {

        int i = 0;
        while (i < inst_set.length()) {

            char inst = inst_set.charAt(i++); //get each inst separately as a character

            if (inst == 'M') {

                Cell nextCell = p.getNeighbour(dir, currentLocation);

                if (nextCell != null)
                    currentLocation = nextCell;

                else
                    System.out.println("This move is not possible. Going to next Instruction");

            }

            if (inst == 'L' || inst == 'R') {

                dir = setDirection(dir, inst);

            }
        }
    }

    public void showCurrentLocation(){

        System.out.println("Current Cell is:");
        System.out.print(currentLocation.x);
        System.out.print("\t"+currentLocation.y);
        System.out.print("\t"+dir+"\n");

    }

    public Direction setDirection(Direction dir, char inst) {

        if(inst=='L') {
            switch (dir) {
                case N:
                    dir = Direction.W;
                    break;
                case W:
                    dir = Direction.S;
                    break;
                case S:
                    dir = Direction.E;
                    break;
                case E:
                    dir = Direction.N;
                    break;
            }
        }

        else if(inst=='R'){

            switch (dir) {
                case N:
                    dir = Direction.E;
                    break;
                case W:
                    dir = Direction.N;
                    break;
                case S:
                    dir = Direction.W;
                    break;
                case E:
                    dir = Direction.S;
                    break;
            }
        }

        return dir;

    }

    public static void main(String... a){

        Plateau p1=new Plateau();
        Scanner in = new Scanner(System.in);

        int x,y;
        String instSet;

        String dir;
        System.out.println("Enter the direction the rover is Headed");
        dir=in.nextLine();

        System.out.println("Enter the Initial XLocation:");
        x=in.nextInt();
        in.nextLine();

        System.out.println("Enter the Initial YLocation:");
        y=in.nextInt();
        in.nextLine();

        System.out.println("Enter the Instructions\t");
        instSet=in.nextLine();

        MarsRover mr1=new MarsRover(p1,x,y,Direction.valueOf(dir),instSet);
        mr1.executeInst();
        mr1.showCurrentLocation();

    }

}

Plateau.java

import java.util.Scanner;
/**
 * Created by Leo on 28-07-2015.
 */

class Plateau {

    private int size;

    public Plateau() {

        Scanner in = new Scanner(System.in);

        System.out.println("Enter the Size of the plateau");
        size=in.nextInt();
        in.nextLine();

    }

    public Plateau(int sz){

        size=sz;

    }

    public Cell getNeighbour(MarsRover.Direction dir,Cell c){

        Cell neighbour = null;

        switch (dir) {

            case N:
                neighbour=new Cell(c.x,c.y+1);
                break;
            case W:
                neighbour=new Cell(c.x-1,c.y);
                break;
            case S:
                neighbour=new Cell(c.x,c.y-1);
                break;
            case E:
                neighbour=new Cell(c.x+1,c.y);
                break;
        }

        if((neighbour.x>=0)&&(neighbour.y>=0)&&(neighbour.x<size)&&(neighbour.y<size))
            return neighbour;

        return null;
    }

}

Cell.java

/**
 * Created by Leo on 28-07-2015.
 */
class Cell{

    int x;
    int y;

    public Cell(int PosX, int PosY) {

        x=PosX;
        y=PosY;

    }

}

MarsRoverTest.java

/**
  * Created by Leo on 24-07-2015.
 */


import org.junit.Test;

import static org.junit.Assert.assertEquals;

public class MarsRoverTest {

    MarsRover.Direction DirIp;
    Plateau p = new Plateau(5);
    MarsRover m = new MarsRover();

    @Test
    public void testRotation() {


        //Assertion For LeftRotation.

        DirIp= MarsRover.Direction.N;
        assertEquals(MarsRover.Direction.W, m.setDirection(DirIp, 'L'));

        DirIp= MarsRover.Direction.W;
        assertEquals(MarsRover.Direction.S, m.setDirection(DirIp, 'L'));

        DirIp= MarsRover.Direction.S;
        assertEquals(MarsRover.Direction.E, m.setDirection(DirIp, 'L'));

        DirIp= MarsRover.Direction.E;
        assertEquals(MarsRover.Direction.N, m.setDirection(DirIp, 'L'));

        //Assertion for RightRotation

        DirIp= MarsRover.Direction.N;
        assertEquals(MarsRover.Direction.E, m.setDirection(DirIp, 'R'));

        DirIp= MarsRover.Direction.W;
        assertEquals(MarsRover.Direction.N, m.setDirection(DirIp, 'R'));

        DirIp= MarsRover.Direction.S;
        assertEquals(MarsRover.Direction.W, m.setDirection(DirIp, 'R'));

        DirIp= MarsRover.Direction.E;
        assertEquals(MarsRover.Direction.S, m.setDirection(DirIp, 'R'));

    }

    @Test
    public void testCheckMovement(){

        //Impossible Moves
        for(int i=0;i<5;i++) {

            DirIp = MarsRover.Direction.W;
            assertEquals(null,p.getNeighbour(DirIp, new Cell(0,i)));

            DirIp=MarsRover.Direction.S;
            assertEquals(null,p.getNeighbour(DirIp,new Cell(i,0)));

        }

        for(int i=0;i<5;i++){

            DirIp=MarsRover.Direction.N;
            assertEquals(null,p.getNeighbour(DirIp,new Cell(i,4)));

            DirIp=MarsRover.Direction.E;
            assertEquals(null,p.getNeighbour(DirIp,new Cell(4,i)));

        }

        //Take a cell from the center and check its moves

        Cell centerCell= new Cell(2,3);
        DirIp=MarsRover.Direction.E;

        Cell next=p.getNeighbour(DirIp,centerCell);

        assertEquals(3,next.x);
        assertEquals(3,next.y);

        DirIp=MarsRover.Direction.N;
        next=p.getNeighbour(DirIp,centerCell);
        assertEquals(2,next.x);
        assertEquals(4,next.y);

        DirIp=MarsRover.Direction.W;
        next=p.getNeighbour(DirIp,centerCell);
        assertEquals(1,next.x);
        assertEquals(3,next.y);

        DirIp=MarsRover.Direction.S;
        next=p.getNeighbour(DirIp,centerCell);
        assertEquals(2,next.x);
        assertEquals(2,next.y);

    }
}

Answer 1

Form/Style

1. Format

In practice, your colleagues will appreciate it if you pay more attention to code formatting. Look into Checkstyle and commonly accepted conventions, such as those defined by Google.

2. Use good variable names.

m is not a good name for a MarsRover, and neither is mr1. Ignoring the fact that these variable names are short and nondescript, the way you abbreviate is inconsistent.

Per, this.dir = dir, you know the "this" keyword. There is a reason this exists, so use it. Now you don't have to take the time to figure out if "size" is sz or size. Just do the following:

public Plateau(int size){
    this.size = size;
}

The same applies to instSet (the constructor parameter) and inst_set (the instance variable [even though you use camelCase format for your other instance variables]) as well as many other variables throughout your code.

You will thank yourself later for using good variable names (even in stub/temporary code). You might not expect anyone to look at this code, but you did post it on Code Review SE.

3. And...

Instead of:

String dir;
dir = in.nextLine();

Just String dir = in.nextLine();.

Function

1. Don't make assumptions, particularly when it comes to user input.

"Enter the Size of the plateau"

How? Is "1x2" valid? How about "3.5?" If you are expecting an integer that describes the length of a square plateau, say so, and better yet, provide an example! Don't assume that the user knows what input is expected.

"Enter the Instructions\t"

Same as above. Also, "\t" is not needed.

2. On the subject of prompts...

"This move is not possible. Going to next Instruction"

Which move? This error message does not help the user fix the problem.

3. Branch properly and validate user input. Take, for example,

if (inst == 'M') { ... } if (inst == 'L' || inst == 'R') { ... }

Now look at the following:

if (inst == 'M') { ... }
else if (inst == 'L' || inst == 'R') { ... }
else { throw new IllegalArgumentException("bad instruction " + inst); }

4. main() does not belong in MarsRover. Put it in a separate class.

6. Use JUnit assertion messages.

MarsRover marsRover;
final MarsRover.Direction NORTH = MarsRover.Direction.N;
...
@Before
public void initialize() {
    marsRover = new MarsRover(new Plateau(5), 1, 1, NORTH, "");
}
@Test
public void testRotation() {
    MarsRover.Direction[] startDirection = new MarsRover.Direction[]{NORTH, EAST, SOUTH, WEST, NORTH, WEST, SOUTH, EAST};
    char[] instSet = new char[]{'R', 'R', 'R', 'R', 'L', 'L', 'L', 'L'};
    for (int i = 0; i < instSet.length; i++) {
        MarsRover.Direction endDirection =  startDirection[(i + 1) % startDirection.length];
        MarsRover.Direction actualDirection = marsRover.setDirection(startDirection[i], instSet[i]);
        assertEquals(instSet[i] + " of " + startDirection[i] + " should be " +
                endDirection + " but is " + actualDirection, endDirection, actualDirection);
    }
}

7. And...

// more work & less readable
private void executeInst() {
    int i = 0;
    while (i < inst_set.length()) {
        char inst = inst_set.charAt(i++);
        ...
// less work & more readable
private void executeInst() {
    for (char inst: instSet.toCharArray()) { ...

Answer 2

It might be overkill to model the whole plateau. That is, your concerns are the rover's location and its heading. It might be worth thinking of this as a problem where the rover is bound to a certain area (the plateau table) and is smart enough to track its own location thereupon. The logic to keep the rover inside its bounds (on the plateau) would move into the rover code itself without any significant increase in its complexity, and Plateau and Cell disappear.