Python solution to Mars Rover

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' a

For once it is nice to see code which is properly commented and for the most parts is easy to read and follow. Kudos to you for that. However, there are stuff which I'm wondering on why you've done as you've done, and with possible suggestions as to how you could tackle those.

Starting from the top:

-
Validate input, not necessarily all parameters – Parameters you pass around would most likely not change by itself to something illegal. As such, it is most common to have the try...except closer to the input.

In your code, you're having try...except both in the actual input method, and also within the actual input loops. Wouldn't it be better to have a dedicated input method which did both cases?

-
Maybe overly fond of try...except? – In general we don't see a lot of try...catch in the code up for review, so it is nice to see that someone is actually thinking about error handling. It does feel like you've taken it a little long, though, and that you need to take it back a step. This is more of a general feeling though, and could be mostly personal opinion.

-
Consider to have the x, y, direction in a dedicated structure – This could possibly benefit from its own class (or structure) just to ease passing it around, and referencing it.

That would also alleviate the need for using the initials array and choice array in some parts of your code.

-
Feature: Potential loop in move(op, r) – If the rover triggers the exception you move it back to start, and start moving again (recursively). What stops it from triggering error another time? And again? And again? And ...

-
Why mars and not planet or plane? – It confuses me when I read about multiple mars'es... :-/

-
Doing input within exception handling is not good – In general, you should avoid doing new input within exception handling as that might trigger other exceptions. The exception handling should be kept to a minimum to clear out the current error situation, and allow for normal operations to preceed (or bail out due to the severity of the exception).

-
Docstring's could be on fewer lines – It's good to have docstrings,
but you take advantage of all the 80 characters available (according to standards), as then they wouldn't take up as much space.

-
The add_rover doesn't actually add a rover – Name methods after what they actually do. This method gets a new legal position and direction for a rover, and doesn't add it anywhere.

It could possibly be named get_rover_initial_position or something like that.

-
Strange logic in add_rover – Having a while True in combination with a return of if rover is not None?, and a possible exit through the check_if_exit method... My head is spinning trying to follow the logic, and it really shouldn't be that hard. Imaging something like:

def get_new_rover(planet):
    """Get a valid position and direction within the planet."""

    rover = None
    while not Rover:

        x, y, direction = get_initial_position("Please enter current rover's initial position and direction");

        try:
            rover = Rover(x, y, direction, planet);
        exception PlanetError as err:
            print(err)

    return rover

It is better, but I still think I would prefer to validate the position instead of triggering an exception within Rover. And I left out the exit handling, which could be done through raising another exceptions or at a higher level or other means. I've got to come back to this one...

-
Avoid flag variables, especially in combination with While true – You use this pattern a few times:

While True:
     flag = False
     ... do something possibly setting: flag = True 
     if flag:
        return

A much better pattern is the following:

flag = False
While not flag:
     ... do something possibly setting: flag = True

And it would be even better if you actually used the parameter you're trying to set instead of a whimsical flag parameter. See rover example above.

-
move_rover does a lot more than just moving the rover – It gets a new operation sequence, tries to move it, updates the planet trail/occupied list and prints a line...

-
Regarding go_end – This method by name goes to the end. It then proceeds with outputting the trail of the rovers, and boom exits. I've never liked a hidden exit in the middle of the code, and would much prefer it to be visible at a much higher level.

-
Why "exit".upper()? – As said before, I'm not entirely thrilled about the the check_if_exit() as it suddenly terminates the program. But why do you not simply write "EXIT" instead of doing it over and over again? (Most likely the compiler will remove this, but it looks a little strange)

Some refactoring

Based upon all of this I would rewrite your main method to something like the following:

```
# Create a planet to move the rovers in
x, y = input_planet_size()
planet = Planet(x, y)

while True:
x, y, direction = input_posi

StackExchange Code Review Q#162988, answer score: 6