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

Python simple battleship game

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

Problem

I'm relatively new to Python, I decided to make a simple battleship game. Is there anything I should do to make it shorter, or in practice, "better"?

from pprint import pprint as pp
import random

Rows = 0
Columns = 0
turns = 0
Answer = "NaN"

print("Welcome to battleship!")

while (Rows > 10) or (Columns > 10) or (Rows  Rows) or (GuessColumn  Columns):
            #Warning if the guess is out of the board
            print("Outside the set grid. Please pick a number within it your Rows and Columns.")

        elif (grid[GuessRow-1][GuessColumn-1] == "X"):
            #If "X" is there than print that it missed
            print("You guessed that already.")

        else:
            #Updates the grid with an "X" saying that you missed the ship
            turns += 1
            print("You missed the ship.")
            update_gridMiss(grid, GuessRow, GuessColumn)
            display_grid(grid, Columns)

    if (turns >= 5):
        print("Game over! You ran out of tries")

Solution

Variable naming

In general, you should follow PEP8, the Python style guide. In particular, it suggests using lowercase for variable names. That is, rows instead of Rows.

User input conditions

while (Rows > 10) or (Columns > 10) or (Rows <= 0) or (Columns <= 0):
    Rows = int(input("Please enter the number of rows you want. \n"))
    Columns = int(input("Please enter the number of columns you want. \n"))


So, if I enter a correct row, but a wrong column, it'll ask again. Without telling me what I did wrong.

I'd suggest

rows = 0
while not (0 < rows <= 10):
    rows = int(input("Please enter the number of rows you want [1-10]."))


And a similar loop for columns. This has one problem: What if the user does not enter a number but something else? 'Pizza' or 'one'. In that case, the int throws an exception, and that should be handled as well.

rows = 0
while not (0 < rows <= 10):
    try:
        rows = int(input("Please enter the number of rows you want [1-10]."))
    except ValueError:
        pass  # Just re-ask the question.


Mixing statements and definitions

The code as given has the form

define function
run some statements
define function
run some statements


In general, you should use

define function
define function
define function
define function main:
    statements
    statements
if __name__ == "__main__":
   main()


Re-using variables

Notice how in create_grid, the variable row has two purposes: It is both a number (due to the for row in range(Rows)), and a list. Please don't do that. Because the number row is unused, we should indicate that to the readers by using the conventional _ as variable name.

def create_grid(Rows, Columns):
    #Creates the 2D Data Grid
    grid = []
    for _ in range(Rows):
        row = []
        for _ in range(Columns):
            row.append(' ')
        grid.append(row)
    return grid


(Note: I myself prefer using a double underscore __ instead, but convention is _.)

Encapsulation

Not sure if you are at the level of classes yet, but I'd suggest learning about them.

Instead of

def create_grid(Rows, Columns):
    #Creates the 2D Data Grid
    grid = []
    for row in range(Rows):
        row = []
        for col in range(Columns):
            row.append(' ')
        grid.append(row)
    return grid

grid = create_grid(Rows,Columns)


You can write

class Grid(object):
    def __init__(self, columns, rows):
        self.columns = columns
        self.rows = rows
        self._grid = []
        for __ in range(rows):
            row = []
            for __ in range(columns):
                row.append(' ')
            self._grid.append(row)


Now, that last part (assigning self._grid) can be done 'nicer' (and faster!).

self._grid = [ [' ' for __ in range(columns)] for __ in range(rows)]


Useless call to .upper.

In display_grid, you've written

column_names = 'abcdefghijklmnopqrstuvwxyz'[:Columns]
    print('  | ' + ' | '.join(column_names.upper()) + ' |')


Why not

column_names = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'[:columns]
    print('  | ' + ' | '.join(column_names) + ' |')


That's a bit nicer, right?

Using enumerate

The builtin enumerate has a second argument: start. You can use it to signal counting should start at 1 instead:

for number, row in enumerate(grid, 1):
       print(number, '| ' + ' | '.join(row) + ' |')


display_grid as a method on the class

If you follow my suggestion of turning the grid into a class, you can write

def display(self):
    """
    Displays a grid.
    """
    column_names = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'[:self.columns]
    print('  | ' + ' | '.join(column_names) + ' |')
    for number, row in enumerate(self._grid, 1):
       print(number, '| ' + ' | '.join(row) + ' |')


But that's just a suggestion.

grid = create_grid(Rows, Columns)
display_grid(grid, Columns)


Docstrings

In Python, it is the convention to add the documentation of a method/function in a docstring.

Instead of

def random_row(grid):
    #Makes a random row integer
    return random.randint(1,len(grid))


write

def random_row(grid):
    """
    Makes a random row integer
    """


(There are also a lot of conventions on how to phrase a docstring. I won't get into that for now.)

Parenthesis in conditional

The code says

while (turns != 5):


It could (and should) be

while turns != 5:


That is, you don't need the ( and ) there.

Weird order in the guess loop

Now, the following is just preference. But to me, the final loop is unclear. I've written it as pseudo-code below.

The final part looks like this (pseudocode)

```
while (turns != 5):
get_guess
if hit:
turns += 1
notify_success
break
else:
if out_of_bounds:
# Print out-of-bounds message
elif already_guessed:
# Print already guessed message
else:

Code Snippets

while (Rows > 10) or (Columns > 10) or (Rows <= 0) or (Columns <= 0):
    Rows = int(input("Please enter the number of rows you want. \n"))
    Columns = int(input("Please enter the number of columns you want. \n"))
rows = 0
while not (0 < rows <= 10):
    rows = int(input("Please enter the number of rows you want [1-10]."))
rows = 0
while not (0 < rows <= 10):
    try:
        rows = int(input("Please enter the number of rows you want [1-10]."))
    except ValueError:
        pass  # Just re-ask the question.
define function
run some statements
define function
run some statements
define function
define function
define function
define function main:
    statements
    statements
if __name__ == "__main__":
   main()

Context

StackExchange Code Review Q#122970, answer score: 9

Revisions (0)

No revisions yet.