First attempt at Mars Rover Kata done in TDD

Question

I have had an attempt at the Mars Rover Kata and would very much appreciate a review and any suggestions to improve.

MessageParser to parse incoming messages and create the domain models:

public class MessageParser
    {
        private static readonly Regex GridDetailsRegex = new Regex(@"\d\s\d");
        private static readonly Regex RoverDetailsRegex = new Regex(@"\d\s\d\s(N|E|S|W)");
        private static readonly Regex MovementDetailsRegex = new Regex(@"(L|R|M)");
        private static readonly string[] LineSeparators = new[] { "\r\n", "\r", "\n" };

        private static readonly Dictionary<char, Movement> MovementsMap =
            new Dictionary<char, Movement>()
            {
                {'L', Movement.TurnLeft},
                {'R', Movement.TurnRight},
                {'M', Movement.Move},
            };

        public string HandleMessage(string message)
        {
            var result = ParseMessage(message);

            return result;
        }

        private Result<string[]> SplitLines(string message)
        {
            if (string.IsNullOrWhiteSpace(message))
            {
                return Result<string[]>.Failure(Error.MessageIsEmpty);
            }

            var lines = message.Split(
                LineSeparators,
                StringSplitOptions.None
            );

            if (lines.Length < 5)
            {
                return Result<string[]>.Failure(Error.MessageIsInvalid);
            }

            var gridDetailsIsValid = GridDetailsRegex.IsMatch(lines[0].Trim());
            var roverParts = lines.Skip(1).ToArray();
            var roverDetailsAreValid = roverParts.Where((rp, index) => index % 2 == 0).All(rd => RoverDetailsRegex.IsMatch(rd));
            var roverMovementsAreValid = roverParts.Where((rp, index) => index % 2 > 0).All(rm=>MovementDetailsRegex.IsMatch(rm));

            return gridDetailsIsValid && roverDetailsAreValid && roverMovementsAreValid ? Result<string[]>.Success(lines) : Result<string[]>.Failure(Error.MessageIsInvalid);
        }

        private string ParseMessage(string message)
        {
            var partsResult = SplitLines(message);

            if (partsResult.IsSuccess == false)
            {
                return Lines(partsResult.Errors);
            }

            var lineParts = partsResult.Value;
            var gridDetails = lineParts.First();
            var grid = Grid.Create(gridDetails);

            if (grid.IsSuccess == false)
            {
                return Lines(grid.Errors);
            }

            var roverDetails = lineParts.Skip(1).ToArray();
            var result = AddRovers(grid.Value, roverDetails);

            return result.IsSuccess ? Lines(grid.Value.GetRoverStatuses()) : Lines(result.Errors);
        }

        private Result AddRovers(Grid grid, string[] roverParts)
        {
            for (int i = 0; i < roverParts.Length - 1; i += 2)
            {
                var rover = Rover.Create(roverParts[i]);
                var movements = roverParts[i + 1].ToArray().Select(m => MovementsMap[m]).ToArray();

                var addResult = grid.AddRover(rover, movements);
                if (addResult.IsSuccess == false)
                {
                    return addResult;
                }
            }

            return Result.Success();
        }

        private static string Lines(IEnumerable<Error> errors)
        {
            return Lines(errors.Select(e => e.Code));
        }

        private static string Lines(IEnumerable<string> lines)
        {
            return string.Join(Environment.NewLine, lines);
        }
    }

Coordinate:

public struct Coordinate
    {
        public int X { get; }
        public int Y { get; }

        public Coordinate(int x, int y)
        {
            X = x;
            Y = y;
        }

        public bool IsWithin(Coordinate minimum, Coordinate maximum)
        {
            return maximum.X >= X && maximum.Y >= Y && minimum.X <= X && minimum.Y <= Y;
        }

        public Coordinate Move(Direction direction)
        {
            if (direction.Equals(Direction.North))
            {
                return new Coordinate(X, Y + 1);
            }

            if (direction.Equals(Direction.East))
            {
                return new Coordinate(X + 1, Y);
            }

            if (direction.Equals(Direction.South))
            {
                return new Coordinate(X, Y - 1);
            }

            return new Coordinate(X - 1, Y);
        }
    }

Direction:

public struct Direction
    {
        public static readonly Direction North = new Direction("N", "E", "W");
        public static readonly Direction East = new Direction("E", "S", "N");
        public static readonly Direction South = new Direction("S", "W", "E");
        public static readonly Direction West = new Direction("W", "N", "S");

        private static readonly List<Direction> ValidDirections = new List<Direction>()
        {
            North,East,South,West
        };

        public string Current { get; }
        public string Right { get; }
        public string Left { get; }

        private Direction(string current, string right, string left)
        {
            Current = current;
            Right = right;
            Left = left;
        }

        public static Direction Create(string direction)
        {
            switch (direction)
            {
                case "N":
                    return North;
                case "E":
                    return East;
                case "S":
                    return South;
                case "W":
                    return West;
                default:
                    throw new ArgumentException($"Invalid direction {direction}", nameof(direction));
            }
        }

        public Direction TurnLeft()
        {
            var left = Left;
            return ValidDirections.Single(d => d.Current == left);
        }

        public Direction TurnRight()
        {
            var right = Right;
            return ValidDirections.Single(d => d.Current == right);
        }

        public override string ToString() => Current;
    }

Movement:

public enum Movement
    {
        TurnLeft,
        TurnRight,
        Move
    }

Rover:

public class Rover
    {
        public string Id { get; }
        public Coordinate Coordinate { get; }
        public Direction Direction { get; }

        private Rover(Coordinate coordinate, Direction direction)
        {
            Coordinate = coordinate;
            Direction = direction;
            Id = $"{coordinate.X}-{coordinate.Y}-{direction}";
        }

        public override string ToString() => $"{Coordinate.X} {Coordinate.Y} {Direction}";

        public Result<Rover> Move(Movement[] movements, Coordinate[] obstacles, Coordinate minimum, Coordinate maximum)
        {
            var newCoordinate = new Coordinate(Coordinate.X, Coordinate.Y);
            Direction newDirection = Direction;

            foreach (var movement in movements)
            {
                if (movement == Movement.Move)
                {
                    newCoordinate = newCoordinate.Move(newDirection);

                    if (newCoordinate.IsWithin(minimum, maximum) == false)
                    {
                        return Result<Rover>.Failure(Error.OutOfBoundRover(Id));
                    }

                    if (obstacles.Any(o => o.Equals(newCoordinate)))
                    {
                        return Result<Rover>.Failure(Error.Obstacle(Id, newCoordinate));
                    }
                }
                else
                {
                    newDirection = movement == Movement.TurnLeft ? newDirection.TurnLeft() : newDirection.TurnRight();
                }
            }

            return Result<Rover>.Success(new Rover(newCoordinate, newDirection));
        }

        public static Rover Create(string roverDetails)
        {
            var parts = roverDetails.Split(" ");

            return new Rover(new Coordinate(int.Parse(parts[0]), int.Parse(parts[1])), Direction.Create(parts[2]));
        }
    }

Grid:

public class Grid
    {
        private readonly Coordinate _maximumBoundaries;
        private readonly Coordinate _minimumBoundaries;
        private readonly List<Rover> _rovers;

        private Grid(Coordinate minimumBoundaries, Coordinate maximumBoundaries)
        {
            _maximumBoundaries = maximumBoundaries;
            _minimumBoundaries = minimumBoundaries;
            _rovers = new List<Rover>();
        }

        public static Result<Grid> Create(string gridDetails)
        {
            var parts = gridDetails.Split(" ");
            var hasX = int.TryParse(parts.FirstOrDefault(), out var gridX);
            var hasY = int.TryParse(parts.LastOrDefault(), out var gridY);

            if (!hasX || !hasY || gridX == 0 && gridY == 0)
            {
                return Result<Grid>.Failure(Error.InvalidGridSize);
            }

            var minimumBoundaries = new Coordinate(0, 0);
            var maximumBoundaries = new Coordinate(gridX, gridY);

            return Result<Grid>.Success(new Grid(minimumBoundaries, maximumBoundaries));
        }

        public Result AddRover(Rover rover, Movement[] movements)
        {
            if (_rovers.Any(r => r.Coordinate.Equals(rover.Coordinate)))
            {
                return Result.Failure(Error.RoverExists(rover.Id));
            }

            if (rover.Coordinate.IsWithin(_minimumBoundaries, _maximumBoundaries) == false)
            {
                return Result.Failure(Error.OutOfBoundRover(rover.Id));
            }

            var obstacles = _rovers
                .Where(r => r.Id != rover.Id)
                .Select(r => r.Coordinate).ToArray();

            var moveResult = rover.Move(movements, obstacles, _minimumBoundaries, _maximumBoundaries);

            if (moveResult.IsSuccess == false)
            {
                return moveResult;
            }

            _rovers.Add(moveResult.Value);

            return Result.Success();
        }

        public string[] GetRoverStatuses()
        {
            return _rovers.Select(r => r.ToString()).ToArray();
        }
    }

Tests:

public class MessageParserTests
    {
        private readonly MessageParser _sut;

        public MessageParserTests()
        {
            _sut = new MessageParser();
        }

        [Fact]
        public void WhenMessageIsValid_InitializesGrid()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be(Lines(
                "1 3 N",
                "5 1 E"));
        }

        [Theory]
        [InlineData("")]
        [InlineData("  ")]
        [InlineData(null)]
        public void WhenMessageIsNullOrEmpty_ReturnsErrorCode(string input)
        {
            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.missing");
        }

        [Fact]
        public void WhenMessageHasMissingGridSize_ReturnsErrorCode()
        {
            var input = Lines(
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageGridSizeTooSmall_ReturnsErrorCode()
        {
            var input = Lines(
                "0 0",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.grid.size.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidXForGridSize_ReturnsErrorCode()
        {
            var input = Lines(
                "x 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidYForGridSize_ReturnsErrorCode()
        {
            var input = Lines(
                "x 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasNegativeGridSize_ReturnsErrorCode()
        {
            var input = Lines(
                "5 -5",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasMissingFirstRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasOutOfBoundFirstRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "6 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.rover.[6-2-N].out.of.bound");
        }

        [Fact]
        public void WhenMessageHasMissingFirstRoverMovements_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidXForFirstRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "X 2 N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidYForFirstRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 Y N",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidDirectionForFirstRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 9",
                "LMLMLMLMM",
                "3 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasMissingSecondRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasOutOfBoundSecondRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 6 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.rover.[3-6-E].out.of.bound");
        }

        [Fact]
        public void WhenMessageHasInvalidXForSecondRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "X 3 E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidYForSecondRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 Y E",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasInvalidDirectionForSecondRoverCoordinates_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 2",
                "MMRMMRMRRM");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasMissingSecondRoverMovements_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "LMLMLMLMM",
                "3 3 E");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.message.invalid");
        }

        [Fact]
        public void WhenMessageHasOverlappingRovers_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "L",
                "1 2 N",
                "R");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.rover.[1-2-N].exists");
        }

        [Fact]
        public void WhenMessageCausesRoversToCrash_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "L",
                "1 1 N",
                "M");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.rover.[1-1-N].met.obstacle.[1,2]");
        }

        [Fact]
        public void WhenMessageCausesRoverToGoOffGrid_ReturnsErrorCode()
        {
            var input = Lines(
                "5 5",
                "1 2 N",
                "MMMM",
                "1 1 N",
                "M");

            var output = _sut.HandleMessage(input);

            output.Should().Be("error.rover.[1-2-N].out.of.bound");
        }

        private static string Lines(params string[] parts)
        {
            return string.Join(Environment.NewLine, parts);
        }
    }

Answer 1

On the whole, I quite like the approach. There's a few things to consider.

Test Size

Your first happy path test seems like it's testing quite a lot more than if the grid is initialized...

[Fact]
public void WhenMessageIsValid_InitializesGrid()
{
   var input = Lines(
       "5 5",
       "1 2 N",
       "LMLMLMLMM",
       "3 3 E",
       "MMRMMRMRRM");
   var output = _sut.HandleMessage(input);

   output.Should().Be(Lines(
       "1 3 N",
       "5 1 E"));
}

It's actually testing if two different rovers can be put on the grid and moved around it, covering all possible direction changes. I'd expect this to be more focussed.

Naming

Naming's always hard, however it's quite important. I'm not sure MessageParser is the right name for your high level class. I'd expect a message parser to take in a message and then return some kind of decoded model to represent the message, for example a list of commands (CreateGrid(5,5), AddRover(1,2,N), MoveRover(id), TurnRover(id, direction) etc). You parser is actually responsible for both decoding the message and executing each of the commands. This feels a bit misleading.

Direction

Your direction class has public three properties Left,Right,Current, which identify the direction string. This is really an implementation detail of the class, I'd consider if these really need to be public, or could they be private. Clients all use the TurnLeft/TurnRight methods.

There's an overlap between the behaviour of TurnLeft, TurnRight and Create. All three methods convert from a string representation (N) to a Direction instance North. Whilst the Turn methods do it via a lookup, Create uses a switch. It seems like this could be consolidated to take one approach (Turn could rely on create, or create could perform a lookup and throw an error if it doesn't exist).

Whilst the list is small, so iterating through it should be fast, I'd still tend to represent this type of lookup as a Map / Dictionary. Using a dictionary, the implementation might look like this:

private static Dictionary<String, Direction> validDirections = new Dictionary<string, Direction>
{
    {"N", North },
    {"E", East },
    {"S", South },
    {"W", West }
};

private static Direction LookupDirection(string encodedDirection)
{
    if (validDirections.TryGetValue(encodedDirection, out Direction direction))
    {
        return direction;
    }
    throw new ArgumentException($"Invalid direction {encodedDirection}", nameof(encodedDirection));
}

public static Direction create(String encodedDirection)
{
    return LookupDirection(encodedDirection);
}

public Direction TurnLeft()
{
    return LookupDirection(Left);
}

public Direction TurnRight()
{
    return LookupDirection(Right);
}