Mars Rover challenge in Python - general feedback

Question

I have given the Mars Rover challenge a go in Python as DSA practice.

Here is the challenge:

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

I am still relatively new to Python so know this is a bit basic - but I wondered if I could get some general feedback on my code for best coding practice?

RIGHT_ROTATE = {
    'N':'E',
    'E':'S',
    'S':'W',
    'W':'N'
}

LEFT_ROTATE = {
    'N':'W',
    'W':'S',
    'S':'E',
    'E':'N'
}



class MarsRover():



    def __init__(self, X, Y, direction):

        self.X = X
        self.Y = Y
        self.direction = direction


    def rotate_right(self):

        self.direction = RIGHT_ROTATE[self.direction]


    def rotate_left(self):

        self.direction = LEFT_ROTATE[self.direction]


    def move(self):

        if self.direction == 'N':
            self.Y += 1
        elif self.direction == 'E':
            self.X += 1
        elif self.direction == 'S':
            self.Y -= 1    
        elif self.direction == 'W':
            self.X -= 1

    def __str__(self):

        return str(self.X) + " " + str(self.Y) + " " + self.direction

    @classmethod
    def get_rover_position(self):

        position = input("Position:")
        X = int(position[0])
        Y = int(position[2])
        direction = position[4]
        return self(X, Y, direction)




class Plateau():



    def __init__(self, height, width):

        self.height = height
        self.width = width


    @classmethod
    def get_plateau_size(self):

        plateau_size = input("Plateau size:")
        height = int(plateau_size[0])
        width = int(plateau_size[2])
        return self(height, width)



def main():

    plateau = Plateau.get_plateau_size()

    while True:

        rover = MarsRover.get_rover_position()        

        current_command = 0 
        command = input("Please input directions for rover.")
        command_length = len(command)

        while current_command <= command_length - 1:

            if command[current_command] == 'L':
                rover.rotate_left()
                current_command += 1
            elif command[current_command] == 'R':
                rover.rotate_right()
                current_command += 1
            elif command[current_command] == 'M':
                rover.move()
                current_command += 1 

        result = str(rover)
        print(result)



if __name__ == '__main__':

    main()

Answer 1

Classes

Typically, a class encapsulates the data and functionality of the thing the class is modeling. The class might have various attributes that describe the thing (position, heading), and some methods for actions the thing does (move). Here, the problem says that NASA sends a command string to the rover. The rover then processes the commands in the command string. So, it makes sense to put the command processing code in a method of the Rover class.

Plateau

In a more complicated simulation, a Plateau (or Terrain) class might make sense for simulating the environment. For example, it could model ground slope or wheel traction, so the rover would require more energy to go uphill or need to go slow on loose soil. In this simulator, it is not needed.

Looping

When processing the command string, it would be more pythonic to iterate over the string directly, rather than using an index into the command string. Instead of

while current_command <= command_length - 1:

        if command[current_command] == 'L':
            rover.rotate_left()
            current_command += 1

        ...

use

for letter in command:

    if letter == 'L':
        ...
    elif letter == 'M':
        ...

I/O

It is generally a good idea to separate I/O from model code. For example, if you wanted to change the current code so the rover is controlled via a web interface, a RESTful API, or via intergalactic WiFi, the Rover class would need to be revised.

f-strings

f-strings makes is easy to format strings. Rather than

str(self.X) + " " + str(self.Y) + " " + self.direction

use

f"{self.X} {self.Y} {self.direction}"

All together, something like this:

RIGHT_ROTATE = {
    'N':'E',
    'E':'S',
    'S':'W',
    'W':'N'
}

LEFT_ROTATE = {
    'N':'W',
    'W':'S',
    'S':'E',
    'E':'N'
}


class MarsRover():
    """
    class to simulate a Mars rover.
    """

    def __init__(self, x, y, heading):
        self.x = x
        self.y = y
        self.heading = heading


    def rotate_right(self):
        """rotate rover 90 degees clockwise."""
        self.direction = RIGHT_ROTATE[self.direction]


    def rotate_left(self):
        """rotate rover 90 degees counter clockwise."""
        self.direction = LEFT_ROTATE[self.direction]


    def move(self):
        """ moves the rover 1 grid square along current heading."""

        if self.heading == 'N':
            self.y += 1
        elif self.heading == 'E':
            self.x += 1
        elif self.heading == 'S':
            self.y -= 1    
        elif self.heading == 'W':
            self.x -= 1


    def execute(self, command_string):
        """parse and execute single letter commands in 
        a command string.

        L/R - turn 90 degrees left/right
        M   - move one grid square in the current heading.
        """

        for command in command_string:
           if command == 'L':
               self.rotate_left()

           elif command == 'R':
               self.rotate_right()

           elif command == 'M':
               self.move()

           else:
               raise ValueError("Unrecognized command '{command"}'."


    def __str__(self):
        return f"{self.x} {self.y} {self.heading}"


def main():
    # this should have some error checking
    coords = input("Enter x and y coordinate (e.g., 3 11): ")
    x, y = (int(s) for s in coords.strip().split())
    heading = input("Enter initial heading: ")

    rover = MarsRover(x, y, heading)        

    while True:

        command_string = input("Please input directions for rover.")
        if comment_string == '':
            break

        rover.execute(command_string)    

        print(str(rover))


if __name__ == '__main__':

    main()

One more thing:

enums

Instead of letters 'N', 'S', etc. consider using enums.

Answer 2

• The Plateau class seems to be pretty unused:
  • It doesn't have actual behaviour, it only hosts the get_plateau_size() method.
  • That method should have a first parameter named cls by convention. self is only used for instances, but here you get the class itself, not an instance.
  • By that, it also returns a Plateau, because calling a class creates an instance. This is kind-of confusing, because I'd have expected some kind of size.
  • Further, the user interaction (input()) was unexpected here. Don't hide such usage details in classes that make sense on their own.
  • The call contains redundancy as well, Plateau.get_plateau_size() features the term "plateau" twice. Once would have been enough.
  • Anyhow, the result isn't even used, so this whole class could be removed!
• LEFT_ROTATE and RIGHT_ROTATE are both only used in a single function, no need to make them globals.
• Take a look at the dataclass decorator ( https://docs.python.org/3/library/dataclasses.html), it could help you structure your classes a bit better. In particular, it would generate __str__ for you.

Answer 3

As a minor nit, I would suggest a final elif with a print "bad command". That way malformed input doesn't go unnoticed.

Answer 4

Input

position = input("Position:")
X = int(position[0])
Y = int(position[2])
direction = position[4]

This input code is fragile. It assumes that the input will always be single digit X, Y locations, always separated by exactly 1 character. If the position is given as 12 15 N, the code would fail converting a space to an integer. Even if that didn’t raise an exception, the X coordinate would be parsed as 1, not 12, and the direction would be 5 instead of N.

Instead, you should split() the input into a list of “terms” based on the white space between them, and parse the first term as the X value, the second term as the Y value, and the third term as the direction:

terms = input("Position:").split()
X = int(terms[0])
Y = int(terms[1])
direction = terms[2]

PEP-8

Variables in Python should be lowercase identifiers (snake_case). The variables and members X and Y should be named x and y.