Mars Rover Kata using TDD and SOLID

Question

I am doing MarsRoverKata exercise just to train my coding skills and I came up with the following solution.

A squad of robotic rovers are to be landed by NASA on a plateau on Mars.

This plateau, which is curiously rectangular, must be navigated by the rovers so that their on board cameras can get a complete view of the surrounding terrain to send back to Earth. A rover's position is represented by a combination of an 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. Assume that the square directly North from (x, y) is (x, y+1).

Input (whether hard coded or input from keyboard): The first line of input is the upper-right coordinates of the plateau, the lower-left coordinates are assumed to be 0,0. The rest of the input is information pertaining to the rovers that have been deployed. Each rover has two lines of input. The first line gives the rover's position, and the second line is a series of instructions telling the rover how to explore the plateau.

The position is made up of two integers and a letter separated by spaces, corresponding to the x and y co-ordinates and the rover's orientation. Each rover will be finished sequentially, which means that the second rover won't start to move until the first one has finished moving. Output: The output for each rover should be its final co-ordinates and heading.

Plateau max X and Y, Starting coordinates, direction and path for two rovers:

 5 5
 1 2 N
 LMLMLMLMM
 3 3 E
 MMRMMRMRRM

Output and new coordinates:

1 3 N
5 1 E

I was trying to follow SOLID principles in my implementation and I used TDD approach to write code. Please criticize.

I have two main classes: MarsRover (manages main parameters of the Rover like initial position and final position) and MarsRoverNavigator (responsible for movements and spinning).

MarsRover.cs:

public class MarsRover
{
    private readonly string input;
    private MarsRoverNavigator marsRoverNavigator;

    public MarsRover(string input)
    {
        this.input = input;
    }

    public NavigationParameters NavigationParameters { get; private set; }
    public string FinalPosition { get; private set; }

    public void Initialize()
    {
        NavigationParameters = InputValidator.GetNaviagtionParametersFromInput(input);
    }

    public void Navigate()
    {
        marsRoverNavigator = new MarsRoverNavigator(NavigationParameters);
        FinalPosition = marsRoverNavigator.Navigate();
    }
}

MarsRoverNavigator.cs:

public class MarsRoverNavigator
{
    private readonly NavigationParameters navigationParameters;
    private SpinningControl spinningControl;
    private MovingControl movingControl;

    public MarsRoverNavigator(NavigationParameters navigationParameters)
    {
        this.navigationParameters = navigationParameters;
        spinningControl = new SpinningControl();
        movingControl = new MovingControl();
    }

    public string Navigate()
    {
        var command = navigationParameters.Command;

        foreach (var step in command)
        {
            DoAStep(step);
        }

        var result = $"{navigationParameters.CurrentCoordinates.X} {navigationParameters.CurrentCoordinates.Y} {navigationParameters.CurrentDirection}";

        return result;
    }

    private void DoAStep(char stepCommand)
    {
        var newDirection = spinningControl.GetNextDirection(navigationParameters.CurrentDirection, stepCommand);

        navigationParameters.UpdateCurrentDirection(newDirection);

        var newCoordinates = movingControl.Move(stepCommand, navigationParameters.CurrentDirection, navigationParameters.CurrentCoordinates);

        if (newCoordinates.X > navigationParameters.PlateauDimenstions.X || newCoordinates.Y > navigationParameters.PlateauDimenstions.Y)
        {
            throw new InvalidCommandException();
        }

        navigationParameters.UpdateCurrentCoordinates(newCoordinates);
    }
}

NavigationParameters.cs:

public class NavigationParameters
{
    public string CurrentDirection { get; private set; }
    public string Command { get; }
    public Coordinates PlateauDimenstions { get; }
    public Coordinates CurrentCoordinates { get; private set; }

    public NavigationParameters(string currentDirection, Coordinates plateauDimenstions, Coordinates currentCoordinates, string command)
    {
        CurrentDirection = currentDirection;
        PlateauDimenstions = plateauDimenstions;
        CurrentCoordinates = currentCoordinates;
        Command = command;
    }

    public void UpdateCurrentDirection(string newDirection)
    {
        CurrentDirection = newDirection;
    }

    internal void UpdateCurrentCoordinates(Coordinates newCoordinates)
    {
        CurrentCoordinates = newCoordinates;
    }

MovingControl.cs is implemented as a dictionary:

public class MovingControl
{
    public Dictionary<string, Func<Coordinates, Coordinates>> MoveFunctions =
                            new Dictionary<string, Func<Coordinates, Coordinates>>
    {
        {"N", MoveNorth},
        {"W", MoveWest},
        {"S", MoveSouth},
        {"E", MoveEast}
    };

    public Coordinates Move(char command, string currentDirection, Coordinates currentCoordinates)
    {
        if (command == 'M')
        {
            return MoveFunctions[currentDirection](currentCoordinates);
        }

        return currentCoordinates;
    }

    private static Coordinates MoveEast(Coordinates coordinates)
    {
        return new Coordinates()
        {
            X = coordinates.X + 1,
            Y = coordinates.Y
        };
    }

    private static Coordinates MoveSouth(Coordinates coordinates)
    {
        return new Coordinates()
        {
            X = coordinates.X,
            Y = coordinates.Y - 1
        };
    }

    private static Coordinates MoveWest(Coordinates coordinates)
    {
        return new Coordinates()
        {
            X = coordinates.X - 1,
            Y = coordinates.Y
        };
    }

    private static Coordinates MoveNorth(Coordinates coordinates)
    {
        return new Coordinates()
        {
            X = coordinates.X,
            Y = coordinates.Y + 1
        };
    }
}

SpinningControl.cs is implemented as a Circular LinkedList:

public class SpinningControl
{
    static readonly LinkedList<string> directions =
                    new LinkedList<string>(new[] { "N", "W", "S", "E" });

    public readonly Dictionary<char, Func<string, string>> SpinningFunctions =
                            new Dictionary<char, Func<string, string>>
    {
        {'L', TurnLeft},
        {'R', TurnRight},
        {'M', Stay }
    };

    public string GetNextDirection(string currentDirection, char stepCommand)
    {
        return SpinningFunctions[stepCommand](currentDirection);
    }

    private static string TurnRight(string currentDirection)
    {
        LinkedListNode<string> currentIndex = directions.Find(currentDirection);
        return currentIndex.PreviousOrLast().Value;
    }

    private static string TurnLeft(string currentDirection)
    {
        LinkedListNode<string> currentIndex = directions.Find(currentDirection);
        return currentIndex.NextOrFirst().Value;
    }

    private static string Stay(string currentDirection)
    {
        return currentDirection;
    }
}

Circular LinkedList extension:

public static class CircularLinkedList
{
    public static LinkedListNode<T> NextOrFirst<T>(this LinkedListNode<T> current)
    {
        return current.Next ?? current.List.First;
    }

    public static LinkedListNode<T> PreviousOrLast<T>(this LinkedListNode<T> current)
    {
        return current.Previous ?? current.List.Last;
    }
}

InputValidator.cs:

public static class InputValidator
{
    private static Coordinates plateauDimenstions;
    private static Coordinates currentCoordinates;
    private static string currentDirection;
    private static string command;

    private static string[] inputByLines;

    private const int expectedNumberOfInputLines = 3;
    private const int expectedLineWithPlateauDimension = 0;
    private const int expectedLineWithStartPosition = 1;
    private const int expectedLineWithCommand = 2;

    private const char linesDelimeter = '\n';
    private const char parametersDelimeter = ' ';

    private static readonly List<string> allowedDirections = new List<string> { "N", "W", "E", "S" };

    public static NavigationParameters GetNaviagtionParametersFromInput(string input)
    {
        SplitInputByLines(input);
        SetPlateauDimensions(inputByLines);
        SetStartPositionAndDirection(inputByLines);
        SetCommand();

        return new NavigationParameters(currentDirection, plateauDimenstions, currentCoordinates, command);
    }

    private static void SplitInputByLines(string input)
    {
        var splitString = input.Split(linesDelimeter);

        if (splitString.Length != expectedNumberOfInputLines)
        {
            throw new IncorrectInputFormatException();
        }

        inputByLines = splitString;
    }

    private static void SetPlateauDimensions(string[] inputLines)
    {
        var stringPlateauDimenstions = inputLines[expectedLineWithPlateauDimension].Split(parametersDelimeter);

        if (PlateauDimensionsAreInvalid(stringPlateauDimenstions))
        {
            throw new IncorrectPlateauDimensionsException();
        }

        plateauDimenstions = new Coordinates
        {
            X = Int32.Parse(stringPlateauDimenstions[0]),
            Y = Int32.Parse(stringPlateauDimenstions[1])
        };
    }

    private static void SetStartPositionAndDirection(string[] inputByLines)
    {
        var stringCurrentPositionAndDirection = inputByLines[expectedLineWithStartPosition].Split(parametersDelimeter);

        if (StartPositionIsInvalid(stringCurrentPositionAndDirection))
        {
            throw new IncorrectStartPositionException();
        }

        currentCoordinates = new Coordinates
        {
            X = Int32.Parse(stringCurrentPositionAndDirection[0]),
            Y = Int32.Parse(stringCurrentPositionAndDirection[1])
        };

        currentDirection = stringCurrentPositionAndDirection[2];
    }

    private static void SetCommand()
    {
        command = inputByLines[expectedLineWithCommand];
    }

    private static bool StartPositionIsInvalid(string[] stringCurrentPositionAndDirection)
    {
        if (stringCurrentPositionAndDirection.Length != 3 || !stringCurrentPositionAndDirection[0].All(char.IsDigit)
           || !stringCurrentPositionAndDirection[1].All(char.IsDigit) || !allowedDirections.Any(stringCurrentPositionAndDirection[2].Contains))
        {
            return true;
        }

        if (Int32.Parse(stringCurrentPositionAndDirection[0]) > plateauDimenstions.X ||
            Int32.Parse(stringCurrentPositionAndDirection[1]) > plateauDimenstions.Y)
        {
            return true;
        }

        return false;
    }

    private static bool PlateauDimensionsAreInvalid(string[] stringPlateauDimenstions)
    {
        if (stringPlateauDimenstions.Length != 2 || !stringPlateauDimenstions[0].All(char.IsDigit)
           || !stringPlateauDimenstions[1].All(char.IsDigit))
        {
            return true;
        }

        return false;
    }
}

Tests around MarsRoverNavigator:

[TestFixture]
public class MarsRoverNavigatorShould
{
    [TestCase("5 5\n0 0 N\nL", "0 0 W")]
    [TestCase("5 5\n0 0 N\nR", "0 0 E")]
    [TestCase("5 5\n0 0 W\nL", "0 0 S")]
    [TestCase("5 5\n0 0 W\nR", "0 0 N")]
    [TestCase("5 5\n0 0 S\nL", "0 0 E")]
    [TestCase("5 5\n0 0 S\nR", "0 0 W")]
    [TestCase("5 5\n0 0 E\nL", "0 0 N")]
    [TestCase("5 5\n0 0 E\nR", "0 0 S")]
    [TestCase("5 5\n1 1 N\nM", "1 2 N")]
    [TestCase("5 5\n1 1 W\nM", "0 1 W")]
    [TestCase("5 5\n1 1 S\nM", "1 0 S")]
    [TestCase("5 5\n1 1 E\nM", "2 1 E")]
    public void UpdateDirectionWhenPassSpinDirections(string input, string expectedDirection)
    {
        var marsRover = new MarsRover(input);
        marsRover.Initialize();
        marsRover.Navigate();

        var actualResult = marsRover.FinalPosition;

        actualResult.Should().BeEquivalentTo(expectedDirection);
    }

    [TestCase("5 5\n0 0 N\nM", "0 1 N")]
    [TestCase("5 5\n1 1 N\nMLMR", "0 2 N")]
    [TestCase("5 5\n1 1 W\nMLMLMLM", "1 1 N")]
    [TestCase("5 5\n0 0 N\nMMMMM", "0 5 N")]
    [TestCase("5 5\n0 0 E\nMMMMM", "5 0 E")]
    [TestCase("5 5\n0 0 N\nRMLMRMLMRMLMRMLM", "4 4 N")]
    public void UpdatePositionWhenPassCorrectInput(string input, string expectedPosition)
    {
        var marsRover = new MarsRover(input);
        marsRover.Initialize();
        marsRover.Navigate();

        var actualResult = marsRover.FinalPosition;

        actualResult.Should().BeEquivalentTo(expectedPosition);
    }

    [TestCase("1 1\n0 0 N\nMM")]
    [TestCase("1 1\n0 0 E\nMM")]
    public void ReturnExceptionWhenCommandSendsRoverOutOfPlateau(string input)
    {
        var marsRover = new MarsRover(input);
        marsRover.Initialize();

        marsRover.Invoking(y => y.Navigate())
                 .Should().Throw<InvalidCommandException>()
                 .WithMessage("Command is invalid: Rover is sent outside the Plateau");
        }
}

Tests around input:

[TestFixture]
public class MarsRoverShould
{
    [TestCase("5 5\n0 0 N\nM", 5, 5, 0, 0, "N", "M")]
    [TestCase("10 10\n5 9 E\nLMLMLM", 10, 10, 5, 9, "E", "LMLMLM")]
    public void ParseAnInputCorrectly(string input, int expectedXPlateauDimension, int expectedYPlateauDimension,
                        int expectedXStartPosition, int expectedYStartPosition, string expectedDirection, string expectedCommand)
    {
        var expectedPlateausDimensions = new Coordinates() { X = expectedXPlateauDimension, Y = expectedYPlateauDimension };
        var expectedStartingPosition = new Coordinates() { X = expectedXStartPosition, Y = expectedYStartPosition };

        var expectedNavigationParameters = new NavigationParameters(expectedDirection, expectedPlateausDimensions, 
                                                    expectedStartingPosition, expectedCommand);

        var marsRover = new MarsRover(input);
        marsRover.Initialize();
        var actualResult = marsRover.NavigationParameters;

        actualResult.Should().BeEquivalentTo(expectedNavigationParameters);
    }

    [TestCase("10 10 5\n1 9 E\nLMLMLM")]
    [TestCase("10\n5 9 E\nLMLMLM")]
    [TestCase("10 A\n5 9 E\nLMLMLM")]
    public void ReturnExceptionWhenWrongPlateauDimensionsInput(string input)
    {
        var marsRover = new MarsRover(input);

        marsRover.Invoking(y => y.Initialize())
            .Should().Throw<IncorrectPlateauDimensionsException>()
            .WithMessage("Plateau dimensions should contain two int parameters: x and y");
    }

    [TestCase("1 1\n1 1\nLMLMLM")]
    [TestCase("1 1\n1 N\nLMLMLM")]
    [TestCase("1 1\n1\nLMLMLM")]
    [TestCase("5 5\n5 A N\nLMLMLM")]
    [TestCase("5 5\n5 1 A\nLMLMLM")]
    [TestCase("1 1\n5 1 N\nLMLMLM")]
    public void ReturnExceptionWhenWrongStartPositionInput(string input)
    {
        var marsRover = new MarsRover(input);

        marsRover.Invoking(y => y.Initialize())
            .Should().Throw<IncorrectStartPositionException>()
                 .WithMessage("Start position and direction should contain three parameters: int x, int y and direction (N, S, W or E)");
    }

    [TestCase("10 10; 5 9; LMLMLM")]
    [TestCase("10 10\nLMLMLM")]
    public void ReturnExceptionWhenWrongInputFormat(string input)
    {
        var marsRover = new MarsRover(input);

        marsRover.Invoking(y => y.Initialize())
            .Should().Throw<IncorrectInputFormatException>()
            .WithMessage("Error occured while splitting the input: format is incorrect");
    }
}

Answer 1

Right off the bat I want to say I love the way you broke everything out into separate classes. I also like that you included your test cases. Both of these will make the code easier to process.

---

Tests

Since this is TDD, lets start with the classes (note: I pulled in NUnit and FluentAssertions from nuget).

I love the use of the TestCase attribute. I don't love the default test name that NUnit gives you when you use it, though. I prefer to set the TestName property of the TestCase attribute, using https://github.com/nunit/docs/wiki/Template-Based-Test-Naming to see all the possibilities. I tend to use something like TestName = {M} <reason for this case>

TestCase also has an ExpectedResult property. You can change the methods to return your expected value, instead of using .Should() assertions. In your case, most of your tests would not benefit, as your expected results are complex types most of the time.

I love that you are also testing your error conditions. Users can clearly see what invalid state looks like.

---

Implementation

Moving onto your implementation (note: I created a Coordinate class with X and Y integer properties. I also created the named exceptions with a hardcoded Message based on your test cases).

MarsRover

Initialize seems like a method that should be private and called by the constructor. Or, Initialize could be called by Navigate and take the input string as parameters. Definitely keep them as separate functions, but from a user perspective I prefer not to call New and Initialize both.

NavigationParameters seems like an implementation detail. I don't know that I would want to expose it as a public property. There is a test that depends on the property (MarsRoverShould.ParseAnInputCorrectly), but that doesn't need to be about the MarsRover object; it could just as easily be scoped into the NavigationParameters object.

MarsRoverNavigator

I like the encapsulation of the moving and spinning movements.

NavigationParameters

PlateauDimenstions should be spelled PlateauDimensions.

MovingControl/SpinningControl

I like the dictionary for determining which direction to travel.

I like the use of a circular list for directions. It's also a good use of extension methods to create a circular list for yourself. Any time of list should work though, it wouldn't have to be linked since it's fixed size the entire time.

Rather than searching directions each time you move, you could be storing the most recent state in the SpinningControl. There would still be an initial search at creation, but after that holding onto the currentDirection state, and just call PreviousOrLast or NextOrFirst.

InputValidator

You start off great with a bunch of constants. But there are other magic numbers later in the file.

currentDirection, command, and inputByLines as static member variables put up a read flag for me. You only have one public method right now, but that could change in the future and suddenly the static state doesn't make sense. I'd prefer to see those passed as parameters everywhere.

allowedDirections has the same list of characters as SpinningControl. I think those could be pulled out into a shared reference. Maybe a CardinalDirections class?

---

General Comments and Final Thoughts

In general, member variables should be as generic as possible. For example, Moving Control.MoveFunctions: instead of using Dictionary, I would recommend IDictionary. Later if you need some change to the implementation, such as using a ReadOnlyDictionary, you can change the internal usage without affecting method signatures.

Again, I really like the way the classes/methods are broken out. And I love that tests are provided. Makes understanding the code from an outsider perspective that much easier.