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

Making a Battleship game

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

Problem

I'm making a Battleship game using python 2.7. However, how to make it is not currently my issue. The main aspect that I'm asking about is the style and method of making the code. Am I following best (or even good) practice? Are the comments too much, too little, unclear? If someone could give me some pointers based on my code on how to make it more readable and standard, it would be highly appreciated.

```
from random import randint

# Adds the board to use as a list later
board = []

# Numbering my rows and columns
row_num = ["0", "1", "2", "3", "4"]

# "0" string begins a space as the row numbering pushed in the output text. Could the space be better implemented in print board?
columns = [" 0", "1", "2", "3", "4"]

for x in range(5):
board.append(["-"] * 5)

# Function for making the board. Added tabs to make it more in the middle and clearer.
def print_board(board):
row_num = 0
print "\t\t", " ".join(columns)
for row in board:
print "\t\t", row_num, " ".join(row)
row_num += 1

print "Let's play Battleship!"
print_board(board)

# Functions for playing agasint the computer
def random_row(board):
return randint(0, len(board) - 1)

def random_col(board):
return randint(0, len(board[0]) - 1)

print "Would you like to play agasint the computer or against a friend?"

# Choosing the location of the battleship and data validation of raw_input
def game_type(game_input):
global ship_row
global ship_col
if game_input == 1:
ship_row = random_row(board)
ship_col = random_col(board)
elif game_input == 2:
ship_row = int(raw_input("Choose a Row to place your battleship:"))
ship_col = int(raw_input("Choose a Column to place your battleship:"))
else:
print "That is not a valid input. Please try again"
game_type(int(raw_input("To play against the computer press 1, to play against a friend press 2: \n")))

game_type(int(raw_input("To play against the computer press

Solution

Reduce global variables

The game_type function can return the ship row and column, there is no need to modify them as global variables, my suggestion is easy to implement:

def game_type(game_input):
    if game_input == 1:
        ship_row = random_row(board)
        ship_col = random_col(board)
    elif game_input == 2:
        ship_row = int(raw_input("Choose a Row to place your battleship:"))
        ship_col = int(raw_input("Choose a Column to place your battleship:"))
    else:
        print "That is not a valid input.  Please try again"
        return game_type(int(raw_input("To play against the computer press 1, to play against a friend press 2: \n")))
    return ship_row, ship_col

ship_row, ship_col = game_type(int(raw_input(
    "To play against the computer press 1, to play against a friend press 2: \n")))


Also, columns is only ever used inside print_board so I would define it as a default parameter, to make it clear that the rest of the program does not need it:

def print_board(board, columns = ["  0", "1", "2", "3", "4"]):
    print "\t\t", "   ".join(columns)
    for row_num, row in enumerate(board):
        print "\t\t", row_num, "   ".join(row)


First define, then "do stuff"

Organization is a nice property of code. Now you mix function definitions and code that "does things" (namely code that interacts with the user).

I would define at first all of the functions, and then run them.

Write a function for input checking

The main logic (now incorporated into main battleship_game function) is full of checks to make sure that the input is valid, abstracting the input checking to another function will condensate the top-level logic.

Costumize

After you define a battleship_game function, you can have fun customizing it by passing all kinds of different parameters: the board size, the number of turns, whether to run in debugging mode or not...

This will allow you to play slightly different battleship games by using the same code.

Use enumerate

99% of the times manually incrementing a counter is not needed in Python:

def print_board(board, columns = ["  0", "1", "2", "3", "4"]):
    print "\t\t", "   ".join(columns)
    for row_num, row in enumerate(board):
        print "\t\t", row_num, "   ".join(row)


Using built-ins is usually simpler than writing your own.

Avoid do-nothing lines

In Python the line:

value


Where value may be anything, does nothing. In your code the line

turn


Does nothing at all, remove it to simplify the code.

List comprehension + _

A list comprehension is preferred over append in simple cases, _ signifies that you are not interested in the value.

board = [["-"] * 5 for _ in range(5)]


Remove unused definitions

row_num is defined but never used, so delete the line:

row_num = ["0", "1", "2", "3", "4"]


Use the range that fits your needs best

>>> help(range)
Help on class range in module builtins:

class range(object)
 |  range(stop) -> range object
 |  range(start, stop[, step]) -> range object
 |  
 |  Return a sequence of numbers from start to stop by step.
 ...


Using the second version of range will avoid incrementing afterwards:

for turn in range(1, 5):
    print "Turn", turn


I also find it more obvious this way that turns are 1-indexed.

Code Snippets

def game_type(game_input):
    if game_input == 1:
        ship_row = random_row(board)
        ship_col = random_col(board)
    elif game_input == 2:
        ship_row = int(raw_input("Choose a Row to place your battleship:"))
        ship_col = int(raw_input("Choose a Column to place your battleship:"))
    else:
        print "That is not a valid input.  Please try again"
        return game_type(int(raw_input("To play against the computer press 1, to play against a friend press 2: \n")))
    return ship_row, ship_col

ship_row, ship_col = game_type(int(raw_input(
    "To play against the computer press 1, to play against a friend press 2: \n")))
def print_board(board, columns = ["  0", "1", "2", "3", "4"]):
    print "\t\t", "   ".join(columns)
    for row_num, row in enumerate(board):
        print "\t\t", row_num, "   ".join(row)
def print_board(board, columns = ["  0", "1", "2", "3", "4"]):
    print "\t\t", "   ".join(columns)
    for row_num, row in enumerate(board):
        print "\t\t", row_num, "   ".join(row)
board = [["-"] * 5 for _ in range(5)]
row_num = ["0", "1", "2", "3", "4"]

Context

StackExchange Code Review Q#115792, answer score: 10

Revisions (0)

No revisions yet.