HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Moving a rover and receiving current coordinates

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
coordinatesrovermovingcurrentreceivingand

Problem

I made a little text-based game where you move a rover and it gives you the current coordinates. You can essentially move around a 1000x1000 grid, and get output into stdout on what your coordinates are. I'm simply wondering if I could improve anything.

# Rover moving program
from random import randint 

# Starting position for rover
r_pos = {'x': randint(1, 1000), 'y': randint(1, 1000)}

def move_rover(x_pos, y_pos):
    if x_pos  1000 and y_pos > 1000:
        print('Invalid position: x{} y{}'.format(
            x_pos, y_pos))


You run it like this.

>>> import rover
>>> rover.move_rover(234, 789)
x234 y789

Solution

Validation

Your validation is weird, and probably not what you intended.

  • move_rover(-1, -1) appears to succeed, even though it's not within your 1000 × 1000 grid.



  • move_rover(-1, 1001) is a no-op. move_rover(1001, 500) is a no-op.



move_rover(2000, 2000) prints a message. However, it would be more idiomatic to raise some kind of ValueError. Printing the error message directly limits the reusability of your code.

Output

Similarly, I advise against having your move_rover() function also print the new coordinates. Each function should do one thing only. Printing should be a separate operation.

There is repetition in the code to format the position in the success and error cases. The formatting code should be factored out to a common routine.

Representation

Using a dictionary with keys named 'x' and 'y' seems cumbersome. You could use a tuple, a namedtuple` or class. A class probably make sense, since your rover should act as an object that responds to messages.

Suggested implementation

from random import randint

MIN_COORD, MAX_COORD = 1, 1000

class Rover(object):
    def __init__(self):
        """Places the rover randomly in the coordinate range with a uniform
        distribution"""
        self.x = randint(MIN_COORD, MAX_COORD)
        self.y = randint(MIN_COORD, MAX_COORD)

    def move(self, x, y):
        if MIN_COORD <= x <= MAX_COORD and MIN_COORD <= y <= MAX_COORD:
            self.x, self.y = x, y
        else:
            raise ValueError('Invalid position: %s' % (self))

    def __str__(self):
        """Reports the position of the rover"""
        return 'x{} y{}'.format(self.x, self.y)


You run it like this:

>>> from rover import Rover
>>> rover = Rover()
>>> print(rover)
x944 y556
>>> rover.move(234, 789)
>>> print(rover)
x234 y789

Code Snippets

from random import randint

MIN_COORD, MAX_COORD = 1, 1000

class Rover(object):
    def __init__(self):
        """Places the rover randomly in the coordinate range with a uniform
        distribution"""
        self.x = randint(MIN_COORD, MAX_COORD)
        self.y = randint(MIN_COORD, MAX_COORD)

    def move(self, x, y):
        if MIN_COORD <= x <= MAX_COORD and MIN_COORD <= y <= MAX_COORD:
            self.x, self.y = x, y
        else:
            raise ValueError('Invalid position: %s' % (self))

    def __str__(self):
        """Reports the position of the rover"""
        return 'x{} y{}'.format(self.x, self.y)
>>> from rover import Rover
>>> rover = Rover()
>>> print(rover)
x944 y556
>>> rover.move(234, 789)
>>> print(rover)
x234 y789

Context

StackExchange Code Review Q#57550, answer score: 6

Revisions (0)

No revisions yet.