6
\$\begingroup\$

I have written the solution of the Mars Rover problem in JavaScript, with a sort of functional approach. I would like to know how I can improve it and if there are any additional edge cases I missed in the tests.

A rover’s position and location is represented by a combination of x and y co-ordinates and a letter representing one of the four cardinal compass points. The plateau is divided up into a grid to simplify navigation. An example position might be 0, 0, N, which means the rover is in the bottom left corner and facing North.

In order to control a rover , NASA sends a simple string of letters. The possible letters are ‘L’, ‘R’ and ‘M’. ‘L’ and ‘R’ makes the rover spin 90 degrees left or right respectively, without moving from its current spot. ‘M’ means move forward one grid point, and maintain the same heading.

Test Input:

5 5

1 2 N

LMLMLMLMM

3 3 E

MMRMMRMRRM

Expected Output:

1 3 N

5 1 E

Dictionary for the rovers to reference depending on the direction they are facing:

const cardinals = {
  N: {
    L: 'W',
    R: 'E',
    move: (x, y)=>{
      return {x:x, y:y+1};
    },
  },
  E: {
    L: 'N',
    R: 'S',
    move: (x, y)=>{
      return {x:x+1, y:y};
    },
  },
  S: {
    L: 'E',
    R: 'W',
    move: (x, y)=>{
      return {x:x, y:y-1};
    },
  },
  W: {
    L: 'S',
    R: 'N',
    move: (x, y)=>{
      return {x:x-1, y:y};
    },
  },
};

The main function that takes the input and gives the desired output:

//mission higher order function
const runMission = (input) => {
  const plateauBounds = processInput.establishBounds(input);
  const rovers = processInput.createRovers(input);
  const directions = processInput.listDirections(input);
  const movedRoversArr = [];
  //iterate over the rovers and each rover's corresponding directions
  for (let i=0; i<directions.length; i++) {
    movedRoversArr.push(executeDirections(directions[i], rovers[i], movedRoversArr));
  }

  for (let j=0; j<movedRoversArr.length; j++) {
    const inBounds = failures.checkBounds(movedRoversArr[j], plateauBounds);
    if (!inBounds) throw 'rover at position ' + j + ' is out of bounds.';
  }

  const result = processOutput(movedRoversArr);
  return result;
};

Functions that parse the input string and prints the output string

//Parsing and printing functions
const establishBounds = (input) => {
  const bounds = {};
  input = input.split('\n')[0].split(' ');
  bounds.x = parseInt(input[0]);
  bounds.y = parseInt(input[1]);
  return bounds;
};

const createRovers = (input) => {
  const rovers = [];
  input = input.split('\n');
  for (let i=1; i<input.length; i=i+2) {
    rovers.push(rover.createRover(input[i]));
  }
  return rovers;
};

const listDirections = (input) => {
  const directions = [];
  input = input.split('\n');
  for (let i=1; i<input.length; i=i+2) {
    directions.push(input[i+1]);
  }
  return directions;
};

const processOutput = (rovers) => {
  let resultString = '';
  rovers.forEach(rover => {
    resultString += `${rover.position.x} ${rover.position.y} ${rover.orientation}\n`;
  });
  return resultString;
};

Functions that transform the string with location and orientation of rover to rover object:

//function to create rover objects
const getLocationValues = (startPos) => {
  const locationValues = {};
  locationValues.x = parseInt(startPos.split(' ')[0]);
  locationValues.y = parseInt(startPos.split(' ')[1]);
  locationValues.orientation = startPos.split(' ')[2];
  return locationValues;
};

const createRover = (startPos) => {  
  const locationValues = getLocationValues(startPos);
  const rover = {
    position: {
      x: locationValues.x,
      y: locationValues.y,
    },
    orientation: locationValues.orientation,
  };

  return rover;
};

Execute directions on one rover checking for collisions and out of bounds:

//function to execute directions on one rover and check for collisions
const executeDirections = (directions, rover, movedRoversArr) => {
  //deep clone the rover
  const movedRover = JSON.parse(JSON.stringify(rover));
  for (let i=0; i<directions.length; i++) {
    const orientation = movedRover.orientation;
    if (directions[i] === 'L') {
      //turn the movedRover left from the cardinal it is currently pointing at
      movedRover.orientation = cardinals[orientation]['L'];
    }
    if (directions[i] === 'R') {
      //turn the movedRover right from the cardinal it is currently pointing at
      movedRover.orientation = cardinals[orientation]['R'];
    }
    if (directions[i] === 'M') {
      //move movedRover forward according to current position
      movedRover.position = cardinals[orientation]['move'](movedRover.position.x, movedRover.position.y);
      failures.checkCollisions(movedRover.position.x, movedRover.position.y, movedRoversArr);
    }
  }
  return movedRover;
};

Check for collisions and out of bounds:

const checkCollisions = (x, y, movedRovers) => {
  if (movedRovers.length > 0) {
    for (let i=0; i<movedRovers.length; i++) {
      if (movedRovers[i].position.x === x && movedRovers[i].position.y === y) {
        throw 'collision detected with rover at position ' + i;
      }
    }
  }
};

const checkBounds = (movedRover, plateauBounds) => {
  if ((movedRover.position.x > plateauBounds.x || movedRover.position.x < 0)
      ||(movedRover.position.y > plateauBounds.y || movedRover.position.x < 0)) {
    return false;
  }
  return true;
};

The Tests:

test('test input matches expected output', () => {
  expect(runMission('5 5\n1 2 N\nLMLMLMLMM\n3 3 E\nMMRMMRMRRM')).toBe('1 3 N\n5 1 E\n');
});

describe('functions to process string input', () => {
  test('establishes bounds with with keys x, y', () => {
    expect(processInput.establishBounds('5 5\n1 2 N\nLMLMLMLMM')).toEqual({'x': 5, 'y': 5});
  });

  test('outputs an array with the correct number of rovers', () => {
    expect(processInput.createRovers('5 5\n1 2 N\nLMLMLMLMM\n3 3 E\nMMRMMRMRRM')).toHaveLength(2);
  });

  test('outputs an array with the correct number of direction sets', () => {
    expect(processInput.createRovers('5 5\n1 2 N\nLMLMLMLMM\n3 3 E\nMMRMMRMRRM')).toHaveLength(2);
  });
});

describe('create rover object from input string', () => {
  test('extracting coordinates as integers and orientation from input', () => {
    expect(rover.getLocationValues('1 2 N')).toEqual({'x': 1, 'y': 2, 'orientation': 'N'});
  });

  test('creates rover object with nested position object and orientation', () => {
    expect(rover.createRover('1 2 N')).toEqual({position: {'x': 1, 'y': 2}, 'orientation': 'N'});
  });
});

describe('checks for failures like collisions and out of bounds', () => {
  test('checks for collisions', () => {
    const runCollision = () => {
      failures.checkCollisions(1, 3, [{position:{x:1, y:3}}]);
    };
    expect(runCollision).toThrow();
  });

  test('check bounds returns false when out of bounds', () => {
    expect(failures.checkBounds({position:{x:5, y:6}}, {x:5, y:5})).toBe(false);
  });

  test('check bounds returns true when rover is in bounds', () => {
    expect(failures.checkBounds({position:{x:3, y:3}}, {x:5, y:5})).toBe(true);
  });
});

describe('execute directions and outputs the final position of the rover', () => {
  test('should not modify original rover', () => {
    const rover = {position: { x: 1, y: 2 }, orientation: 'N'};
    executeDirections('LMLMLMLMM', rover, []);
    expect(rover).toEqual(rover);
  });

  test('turns rover right and left', () => {
    const rover = {position: { x: 1, y: 2 }, orientation: 'N'};
    expect(executeDirections('L', rover, [])).toEqual({position: { x: 1, y: 2 }, orientation: 'W'});
    expect(executeDirections('R', rover, [])).toEqual({position: { x: 1, y: 2 }, orientation: 'E'});
  });

  test('moves rover forward according to current orientation', () => {
    const rover = {position: { x: 1, y: 2 }, orientation: 'N'};
    expect(executeDirections('M', rover, [])).toEqual({position: { x: 1, y: 3 }, orientation: 'N'});
  });
});
\$\endgroup\$
  • 1
    \$\begingroup\$ Your code is pretty solid. By looking at it i only noticed four things to consider. In getLocationValues you split on the same input 3 times, maybe const locationSplits = startPos.split(' ') would be a good idea, in checkBounds you can just say return !statement, rather than if else. Also you could change the arguments executeDirections to an object instead of parameters, that way you could add functions like dig and so on without having too many parameters. Maybe i missed it, but you don't check bounds after moving the rover. \$\endgroup\$ – Pavlo Nov 22 '17 at 23:56
  • 2
    \$\begingroup\$ Not a full answer, but you should throw errors, not strings. \$\endgroup\$ – Gerrit0 Nov 23 '17 at 4:16
  • \$\begingroup\$ Where is processInput declared? \$\endgroup\$ – Alex Keeley Apr 29 at 15:00
4
\$\begingroup\$

One thing you may want to consider is remove the hard coding for 'L' and 'R' instructions. Instead, you can pick up current orientation's proper new orientation by the letter itself.

const executeDirections = (directions, rover, movedRoversArr) => {
  //deep clone the rover
  const movedRover = JSON.parse(JSON.stringify(rover));
  directions.forEach(directionLetter => {
    if (directionLetter === 'M') {
      movedRover.position = cardinals[orientation]['move'](movedRover.position.x, movedRover.position.y);
      failures.checkCollisions(movedRover.position.x, movedRover.position.y, movedRoversArr);
    } else {
      movedRover.orientation = cardinals[orientation][directionLetter];
    }
  });
  return movedRover;
};

This has a fundamental difference (basically, it's following the idea of "Strategy" design pattern). Think, what the if-based code would look like if NASA says: "Oh, we want rover to support A direction for turning 45 degrees left, and B for 45 degrees right, and R to fully reverse -- 180 degrees!"

With what I'm suggesting, only the cardinals object would need to change, not the executeDirections function...

\$\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.