Mars Rover Challenge in C#

Question

This is my code for the Mars Rover Challenge, would be nice to get some opinions on it. More details about what the Mars Rover challenge is can be found here:

https://code.google.com/archive/p/marsrovertechchallenge/

The Mars Rover takes an IEnumerable<string> of instructions, the first element is the size of the surface, "x y" where x is x is length and y is width. Second string element is the starting position, "1 2 N" where N is the direction facing. N being north, S being south etc. Finally the last element is a set of instructions, "LMLMLMLMM" where L is left M is move and R is right.

Code should output final position, "1 3 N".

See code below with test cases:

using System.Collections.Generic;
using System.Linq;
using MarsRover.Domain.Models;
using MarsRover.Models.Domain;
namespace MarsRover.Logic.Services
{
 public class NavigationService
 {
 protected readonly IReadOnlyCollection<(char FacingDirection, char Instruction, char NewDirection)> DirectionLookup
 = new List<(char FacingDirection, char Instruction, char NewDirection)>
 {
 ('N', 'L', 'W'),
 ('W', 'L', 'S'),
 ('S', 'L', 'E'),
 ('E', 'L', 'N'),
 ('N', 'R', 'E'),
 ('E', 'R', 'S'),
 ('S', 'R', 'W'),
 ('W', 'R', 'N')
 };
 /// <summary>
 /// Navigate Mars Rover using set of instructions
 /// </summary>
 /// <param name="instructions"></param>
 /// <returns>String of the Mars Rovers final coordinates and the direction it's facing.</returns>
 public virtual string Navigate(IList<string> instructions)
 {
 var parsedInstructionsTuple = ValidateAndParseInsutrctions(instructions);
 if (!parsedInstructionsTuple.isValid) return string.Empty;
 var xLength = parsedInstructionsTuple.parsedInstructions.Plateau.XTotalLength;
 var yLength = parsedInstructionsTuple.parsedInstructions.Plateau.YTotalLength;
 var xStartingPosition = parsedInstructionsTuple.parsedInstructions.StartingPosition.XStartingPosition;
 var yStartingPosition = parsedInstructionsTuple.parsedInstructions.StartingPosition.YStartingPosition;
 var facingDirection = parsedInstructionsTuple.parsedInstructions.StartingPosition.DirectionFacing;
 var position = SetStartingPosition(xStartingPosition, yStartingPosition, facingDirection);
 foreach (var instruction in parsedInstructionsTuple.parsedInstructions.MovementInstructions)
 {
 position = Move(instruction, xLength, yLength, position);
 }
 return $"{position.XPosition} {position.YPosition} {position.FacingDirection}";
 }
 /// <summary>
 /// Turn or move Mars Rover based on the current instruction
 /// </summary>
 /// <param name="instruction"></param>
 /// <param name="currentPosition"></param>
 /// <returns></returns>
 protected virtual Position Move(char instruction, int xLength, int yLength, Position currentPosition)
 {
 if (instruction != 'M')
 {
 var lookupValue = DirectionLookup
 .FirstOrDefault(lookup => lookup.FacingDirection == currentPosition.FacingDirection.ToCharArray().FirstOrDefault() && lookup.Instruction == instruction);
 currentPosition.FacingDirection = lookupValue.NewDirection.ToString();
 return currentPosition;
 }
 var movementAxis = currentPosition.FacingDirection == "E" || currentPosition.FacingDirection == "W" ? "X" : "Y";
 var xPosition = currentPosition.XPosition;
 var yPosition = currentPosition.YPosition;
 if (movementAxis == "X")
 {
 xPosition = currentPosition.FacingDirection == "W" ? currentPosition.XPosition - 1 : currentPosition.XPosition + 1;
 }
 else
 {
 yPosition = currentPosition.FacingDirection == "S" ? currentPosition.YPosition - 1 : currentPosition.YPosition + 1;
 }
 return new Position { FacingDirection = currentPosition.FacingDirection, XPosition = xPosition, YPosition = yPosition };
 }
 /// <summary>
 /// Set starting position that Mars Rover will navigate from.
 /// </summary>
 /// <param name="x"></param>
 /// <param name="y"></param>
 /// <param name="facingDirection"></param>
 /// <returns>Position object with x and y coordinates and is occupied flag.</returns>
 protected Position SetStartingPosition(int x, int y, string facingDirection)
 {
 return new Position { XPosition = x, YPosition = y, FacingDirection = facingDirection };
 }
 /// <summary>
 /// Helper method to parse and validate Mars Rover inputs.
 /// </summary>
 /// <param name="instructions"></param>
 /// <returns>Boolean based on validation of instructions and tuple of instructions for navigation.</returns>
 protected virtual (bool isValid, ParsedInstructions parsedInstructions) ValidateAndParseInsutrctions(IList<string> instructions)
 {
 if (!instructions.Any() || instructions.Count != 3)
 return (isValid: false, parsedInstructions: null);
 var plateau = instructions[0].Split(' ');
 if(plateau.Length != 2)
 return (isValid: false, parsedInstructions: null);
 var startingPosition = instructions[1].Split(' ');
 var movementInstructions = instructions[2].ToArray();
 var invalidInputsList = new List<bool>
 {
 int.TryParse(plateau[0], out var xTotalLength),
 int.TryParse(plateau[1], out var yTotalLength),
 int.TryParse(startingPosition[0], out var xStartingPosition),
 int.TryParse(startingPosition[1], out var yStartingPosition)
 };
 if (plateau.Length != 2 || startingPosition.Length != 3 || invalidInputsList.Contains(false))
 return (isValid: false, null);
 return (isValid: true, parsedInstructions: new ParsedInstructions
 {
 Plateau = (xTotalLength, yTotalLength),
 StartingPosition = (xStartingPosition, yStartingPosition, startingPosition[2].FirstOrDefault().ToString()),
 MovementInstructions = movementInstructions
 });
 }
 }
}

Test cases

using Xunit;
using MarsRover.Logic.Services;
namespace MarsRover.UnitTests
{
 public class NavigationServiceTests
 {
 [Theory]
 [InlineData(new string[] { "5 5", "1 2 N", "LMLMLMLMM" }, "1 3 N", "Rover navigated to expected destination.")]
 [InlineData(new string[] { "5 5", "3 3 E", "MMRMMRMRRM" }, "5 1 E", "Rover navigated to expected destination.")]
 [InlineData(new string[] { }, "", "Invalid input because of empty string array")]
 [InlineData(new string[] { "5 5", "3 3", "MMRMMRMRRM" }, "", "Invalid input because the starting destination doesn't have direction.")]
 [InlineData(new string[] { "5 5", "3 3", "WASDRRRLLLSDD" }, "", "Invalid input because of invalid characters.")]
 [InlineData(new string[] { ",,,,rrrr", "3 3", "MMRMMRMRRM" }, "", "Invalid input because of invalid characters.")]
 [InlineData(new string[] { "5", "3 3", "MMRMMRMRRM" }, "", "Invalid input because no x or y position to create surface array.")]
 [InlineData(new string[] { "5 5", "1 2 N", "" }, "1 2 N", "Rover stayed in the same position.")]
 public void NavigateSurfaceTest(string[] input, string expected, string reason)
 {
 // Arrange
 var SUT = new NavigationService();
 // Act
 var actual = SUT.Navigate(input);
 // Assert
 Assert.Equal(expected, actual);
 }
 }
}

Full code is here: https://github.com/NickGowdy/MarsRoverChallenge

• AlexV: Not everyone (including me) knows what the "Mars Rover Challenge" is. Include at least a short description of the problem directly in your question and link to a full problem description if possible.
• nick gowdy: @AlexV I've added the link of the test to my post.
• pacmaninbw: It would have been better to add at least some of the description of the problem as well as the link since links may go bad.
• Wim ten Brink: I would have liked to see a Grid class that would check if any position is within the grid or not. And I would have liked to see a Rover class with a Move() and Turn() method, which would be linked to the grid. You could then have the rover check if it's still in the grid and has not collided with another rover.

Answer 1

Overall this looks fine. There are some small things that I would note, though.

• If you deconstruct the return value from ValidateAndParseInsutrctions (typo, btw) like so:

  var (valid, instructions) = ValidateAndParseInstructions(...);
  

  you can have shorter assignments afterwards:

  if (!valid) return string.Empty;
  var xLength = instructions.Plateau.XTotalLength;
  var yLength = instructions.Plateau.YTotalLength;
  // ...
  

  which reads a bit nicer.

• I don't really see the need for SetStartingPosition. You're not setting a position, you're creating and returning a Position. This method is basically a wrapper around the Position constructor.

• I don't like how Move sometimes mutates the input currentPosition and sometimes returns a new Position. If I were you, I'd make Position immutable and always return a new one. As pointed out in the comments, if you were to make the class immutable, it would also be a good idea to rename it to Location.

• In this piece of code:

  var lookupValue = DirectionLookup
   .FirstOrDefault(lookup => lookup.FacingDirection == currentPosition.FacingDirection.ToCharArray().FirstOrDefault() 
   && lookup.Instruction == instruction);
  

  you have the problem of mixing char (your method input) and string (your lookup). You should decide whether you want to represent instructions as char or string and then stick to it.

  var lookupValue = DirectionLookup.FirstOrDefault(lookup =>
   lookup.FacingDirection == currentPosition.FacingDirection
   && lookup.Instruction == instruction);
  

• Related to that, I'm wondering if it makes sense to replace the literals with const values (e. g. private const string Move = "M";).

• I just saw that your Navigate method takes an IList<string> instead of an IEnumerable<string> like your problem statement says. As you do not need anything from the IList<T> interface, I'd say you can simply accept an IEnumerable<string> here.

• Flater: "If I were you, I'd make Position immutable and always return a new one." I agree with the technical reasoning, but slightly balk at the naming. Semantically, a position can change, whereas a location cannot. I would say that a position is a property of the rover (which changes as it moves around) whereas a location is a type (it represents a given coordinate). If you make Position immutable, I would suggest renaming it to Location (or Coordinate, or Point, or ...) to more accurately reflect what it represents.
• germi: That's a good point. I'll update the answer.