\$\begingroup\$

main.py

from Rover import Rover from Mars import Mars

According to PEP 8 (the official style guide for Python):

Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged.

(https://www.python.org/dev/peps/pep-0008/#package-and-module-names)

""" """

Now that doesn't do much good, does it? That's meant to be a documentation string, but it's blank. Either write some documentation, or don't put those quotation marks there. Somebody might think you're writing in invisible ink.

if x.isdigit() and y.isdigit():

Python is smarter than you are with integers. That's no insult; Python is pretty smart. You may think that a string that has something other than a digit is not convertable to an integer, but that is not true (I'm pretty sure). I actually have failed to come up with an example where .isdigit() fails yet int() can come up with something, but I'm pretty sure there is such a case. Besides that, EAFP (Easier to Ask Forgiveness than Permission) is a more commonly preffered method in Python than LBYL (Look Before You Leap) because it tends to take fewer function calls (as is true here) and you're less likely to have a race condition in multi-threaded code. I would do that here:

try: mars = Mars(int(x), int(y)) Rover.Mars = mars return True except ValueError: return False

I also took out the print call because after all this is just a validation function. I would leave it up to the calling function whether or not it should do someting about it.

I'm also not liking the Rover.Mars = ... bit. What if you want more than one Mars depending on the rover? In my opinion, that should be an instance attribute, not a class attribute.

if x.isdigit() and y.isdigit() and direction.isalpha(): return True return False

You just defined a function to validate x and y . Why not use it? Since I took out that print call, it can be used here just fine without the user knowing anything about it. As for direction , just being alphabetic does nothing for you. It needs to be one of the direction letters. Next, I have a problem with using an if check in the first place. if executes something if a block of code evaluates to something "truthy". Instead of returning True when something is truthy and False when it isn't, why not just return the truthy thing in the first place? Well, okay; some truthy things are not suitable return values, but here they aren't just truthy or falsey; they are True or False:

from Rover import Rover, directions # <- added directions import ... def validate_rover(x, y, direction): return validate_mars(x, y) and direction in directions

def validate_operations(op): import re

According to PEP 8:

Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.

(https://www.python.org/dev/peps/pep-0008/#imports)

Put the import up at the top, not in the function. I don't think there's actually any performance difference, but it's cleaner.

while (flag == False):

I have three problems with this line:

flag is not at all a descriptive name. Try to give it a name that says why there's a flag here. Python does not use parentheses like JavaScript does. They're just extra here. There's no need to compare to False. The while condition works only with booleans, so there's no need to create another boolean based on this boolean's comparison; just reverse the boolean: while not flag:

The same goes for the next loop.

Besides this, I don't really like the looks of that loop. I would do something like this:

while True: choice = input("...").split() if len(choice) == 2 and validate_mars(*choice): break print("Incorrect input...")

Come to think of it, I don't like that you need to do a length check here. I would prefer if validate_mars() would take an iterable as its only argument and do the length check itself.

exit()

I would prefer if this were a simple return . That way, if for example another file imported this and called main() , it could still do other things after main() is done. It's a matter of opinion, but I prefer letting the calling functions make as many decisions as possible. Defaults are good, but I like to make a lot of it be configurable.

Mars.py

def __init__(...): ... print(...)

Printing is something I think should be left to the calling function. If that's what it wants to say when Mars emerges, so be it; but I don't think Mars should be the one to decide.

def getX(...): ... def getY(...):

To quote PEP 8:

Function names should be lowercase, with words separated by underscores as necessary to improve readability. mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

(https://www.python.org/dev/peps/pep-0008/#function-names)

That is also relevant in several of the methods in Rover.py.

Rover.py

directions = [...]

A list is helpful when you need to maintain a specific order and you need to be able to change it later, but you don't need to change anything for this. That's why I prefer a tuple. You can't use the more-efficient set because order is important here, but the immutable tuple works just fine.

def __init__(...): ... print(...)

This is the same as in Mars.py. I prefer if the calling function does the printing.