I'm still quite new to Python, but am loving the language (I come from a more strongly-typed language background..).
A month or so ago I found the Mars Rover Challenge and attempted to solve it. I quite quickly managed to solve the problem, but in all honesty I can't help but feel like there is a lot of code-smells in my solution.
Could I get some help on ways to refactor functions and (if possible) add polymorphism to the classes to make it easier to extend on them?
Overall feedback would be great too!
main.py
#!/usr/bin/env python3
from Rover import Rover, directions
from Mars import Mars
import re
def validate_int(x, y):
"""
Validate that the two parameters are integers.
Re-used multiple times during the execution.
"""
try:
if int(x) >= 0 and int(y) >= 0:
return True
except Exception as err:
print("Only numerical elements. Try again!")
return False
def validate_rover(x, y, direction):
"""
Validates a rover to ensure the parameters are
correct datatypes(int, int, string).
It also controls ensures that the integers
are inside the Mars surface and that the
supplied direction is a correct direction.
"""
try:
if validate_int(x, y) and direction in directions:
return True
except ValueError as err:
print("Error: {}\n. Please enter two numbers followed by a char either N, E, S or W\nTry again!".format(err))
return False
print("You seem to have entered an incorrect Rover.\nPlease enter two numbers followed by a char either N, E, S or W\nTry again!\n")
return False
def validate_operations(op):
"""
Uses regex to validate that
the supplied string only contains
'M', 'R' and 'L'.
Raises a ValueError if incorrect
operation(s) have been supplied.
"""
pattern = re.compile("^[MRL]*$")
if pattern.match(op):
return True
else:
raise ValueError("Only values 'L', 'M' or 'R' accepted!")
def move(op, r):
"""
Uses the supplied operations
and moves the rover according to
the string of operations.
If a rover goes out of bounds it is
returned to its initial position
(where it was initialized at).
"""
try:
for operation in op:
if operation == "L":
r.turnLeft()
elif operation == "R":
r.turnRight()
else:
r.forward()
except Exception as err:
op = input("Error: {}\nReturning it to inital position ({}, {} facing {}).\
Try again!\n>>> ".format(err, r.initial[0], r.initial[1], r.initial[2]))
r.return_to_start()
move(op, r)
def add_rover(mars):
"""
Taking a reference to Mars
this function asks for user input.
The user input is then validate.
If it passes validation a new Rover
is created and returned.
If the input doesn't pass validation
the user is prompted to enter a
new Rover.
"""
while True:
rover = None
try:
choice = check_if_exit(input("Please enter the current rover's initial position.\nRemember to keep inside Mars limits!\n>>> "), mars).split()
if len(choice) == 3 and validate_rover(choice[0], choice[1], choice[2]): # Check the length of supplied input and check type-integrity
rover = Rover(int(choice[0]), int(choice[1]), choice[2], mars) # Initiate a rover with the supplied input. Each rover is assigned to a Mars (many-to-one relation).
except ValueError as err:
print(err)
continue
if rover is not None:
return rover
def move_rover(rover, mars):
"""
Taking the created rover and mars
the function then asks for user input.
The input is validated and then
sent to move() which then performs
the operations.
The Rover is then added to the
occupied spaces on Mars and a
new Rover will be prompted.
"""
while True:
moved = False
try:
choice = check_if_exit(input("Enter a sequence of operations.\n>>> "), mars).upper()
if validate_operations(choice): # Validate that the supplied operations are 'L', 'M' or 'R'
move(choice, rover) # Peform the moves on the rover
mars.occupied.append((rover.x, rover.y, rover.direction))
#print(mars.occupied)
moved = True
print("\n--------------------------------------\n")
except Exception as err:
print(err)
continue
if moved:
return
def go_end(mars):
"""
This function simply gives a pretty output
of all the Rovers on Mars surface.
Then exits the application.
"""
print("--------------- Output ---------------")
for rover in mars.occupied:
print("{} {} {}".format(rover[0], rover[1], rover[2]))
print("--------------------------------------")
print("Exiting application.")
exit()
def check_if_exit(iput, mars):
"""
Every input taken after setting up Mars will
use this function to check if the input
contains 'exit'.
If it contains 'exit' it the go_end
function will be called.
"""
if "exit".upper() in iput.upper():
go_end(mars)
return iput
def main ():
"""
The brains of the code.
Uses a few while loops to validate
user input and to make the code more user friendly.
An exit when prompted will print the rover's positions.
"""
inactivate_loop = False
while (not inactivate_loop): # will loop until Mars has been initialized.
choice = input("Please enter the size of mars (2 numbers, separated by a space and higher than 0.)\n>>> ").split()
if len(choice) == 2 and validate_int(choice[0], choice[1]): # Checks so that the input is length 2 and then checks type-integrity
mars = Mars(int(choice[0]), int(choice[1]))
inactivate_loop = True
else:
print("Incorrect input. Please ensure you enter a string with two numerical elements")
print("\n--------------------------------------\n")
while True:
rover = add_rover(mars) # Initiate a rover with the supplied input. Each rover is assigned to a Mars (many-to-one relation).
move_rover(rover, mars)
if __name__ == "__main__":
main()
Mars.py:
class Mars(object):
"""
As mentions above, creates a Mars-object.
"""
def __init__(self, x, y):
"""
Initializes a Mars-object.
"""
self.x = x
self.y = y
self.occupied = []
Rover.py
"""
The Rover class handles everything concerning the rover.
Meaning it handles the initialization of new rovers,
the movement and resetting of it.
A rover is initialized with two integers (its x- and y-values)
a direction (N, E, S, W) and what Mars it is related to (a many-to-one relationship).
The reason I added Mars to the initialization is mainly due to the fact
that it would be easy to add multiple rectangles if that was in the test-case
and at the same time move a Rover from one Mars to another.
It also makes it easier when validating the Rover to its assigned Mars as
we can ensure that it is in that particular Mars size.
I also decided to use the operator library as I wanted to show
that I understand the concept of working with getters and setters.
Even though that isn't necessary to use in Python it does make
the validation of attributes a bit 'neater' in my opinion.
"""
import operator
#
directions = ("N", "E", "S", "W") # Use tuple since we don't need to manipulate
# the direciton after initializing it (immutable)
class Rover(object):
"""
As mentioned above, creates a Rover-object.
"""
def __init__(self, x, y, direction, mars):
"""
Initialize a rover
"""
self.Mars = mars
self.direction = direction
self.x = x
self.y = y
self.initial = (self._x, self._y, self._direction)
direction = property(operator.attrgetter('_direction'))
"""
Setters, uses basic validation to ensure type-integrity.
In directions uses tuple to ensure that direction is indeed correct.
"""
@direction.setter
def direction(self, d):
"""
Checks if the direction exists in the
defined directions.
If the direction doesn't exist a
ValueError is raised.
Otherwise the direction is set.
"""
if d.upper() not in directions:
raise ValueError("Direction not correct, use 'N, E, S or W'")
self._direction = d.upper()
x = property(operator.attrgetter('_x'))
@x.setter
def x(self, x):
"""
Checks if x is within acceptable
bounds (higher than 0 and lower than
Mars surface)
"""
if (x < 0 or x > self.Mars.x):
raise ValueError("""This rover's x-value is out of bounds.
It should be value should be < 0 and > {}""".format(str(self.Mars.x)))
self._x = x
y = property(operator.attrgetter('_y'))
@y.setter
def y(self, y):
"""
Checks if y is within acceptable
bounds (higher than 0 and lower than
Mars surface).
"""
if (y < 0 or y > self.Mars.y):
raise ValueError("This rover's y-valueis out of bounds.\
It should be value should be < 0 and > " + str(self.Mars.y))
self._y = y
initial = property(operator.attrgetter('_initial'))
@initial.setter
def initial(self, tup):
"""
Checks if the two first items in the
passed tupple tup exist
in the occupied spaces in the rover's
assigned Mars.
If it does exist that means the
space is already taken and an error
is raised.
If the space is empty, the space is
set to the initial space (used
if the Rover goes out of bounds).
"""
for spaces in self.Mars.occupied:
if (tup[0], tup[1]) == (spaces[0], spaces[1]): raise RuntimeError("This position is \
already occupied by another rover")
#if (tup[0], tup[1]) in self.Mars.occupied: raise IndexError("This position is already occupied by another rover")
self._initial = tup
def get_formatted_position(self):
"""
Returns a formatted string containing the
Rover's position (as used in output).
"""
return "{} {} {}".format(self._x, self._y, self._direction)
def get_current_position(self):
"""
Get the current position of the rover.
Usually we don't need getters in Python,
but I wanted to return a formatted string.
This is also used in the unittest-cases to
assert the position of the Rover.
"""
return (self._x, self._y)
"""
Re-initializes the object's positioning
in case the rover goes out of bound.
"""
def return_to_start(self):
self.x = self._initial[0]
self.y = self._initial[1]
self.direction = self._initial[2]
"""
Movement functions used to move the rover.
"""
def turnRight(self):
"""
Checks if the current direction is in the
end of the tuple.
If it's in the end of the
tuple we take the first item in the tuple
to be the new direction.
Otherwise we take the element on index + 1.
Basically attempts to mimic a compass by
simulating a 'circular' list.
"""
self.direction = directions[0] \
if directions.index(self._direction) == 3 \
else directions[directions.index(self._direction) + 1]
def turnLeft(self):
"""
Does the opposite of turnRight
by checking if the item is in
the beginning of the list.
"""
self.direction = directions[3] \
if directions.index(self._direction) == 0 \
else directions[directions.index(self._direction) - 1]
def forward(self):
"""
Move the Rover forward in the direction it is facing
by setting its Y- or X-value.
This will use the class setters, so if the
Rover goes outside of bounds it will raise an
exception, which in turn will reset its position!
It also raises an exception in case a forward Movement
means it hits another rover already occupying its
new space.
"""
if self._direction == "N":
self.y = self._y + 1
elif self._direction == "S":
self.y = self._y - 1
elif self._direction == "E":
self.x = self._x + 1
else:
self.x = self._x - 1
#Test the current space
for spaces in self.Mars.occupied:
if self.get_current_position() == (spaces[0], spaces[1]):
raise RuntimeErrorjk("I've hit a position that is already taken.\n\
Returning to my initial position. Please try again!\n")
self.return_to_start()
