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

Simple 1-player Battleships game

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

Problem

This is my simple code for a Battleships game:

```
from time import sleep
from random import randint
from math import sqrt
global yes, no, board, length, height, area, gamesPlayed, \
wins, shipRow, shipCol, guessRow, guessCol, look
yes = ["yes", "y", "yeah", "yh", "sure", "again",
"uh-huh", "definitely", "why not?", "yes.", "yes!"]
no = ["no", "n", "nope", "hate you", "stop", "never",
"stop it", "stahp eet", "why would i?"]
gamesPlayed = 0
wins = 0
inp = input("Would you like to know how to play? ").lower()

if inp in yes:
sleep(1)
print("A battle ship is hidden in a patch of ocean.")
sleep(1)
print("You have some bombs to try to hit it.")
sleep(1)
print("To deploy a bomb, we need to know where to launch it")
sleep(1)
print("The most top left area is row 1, column 1.")
sleep(1)
print("You can make it harder by having less turns and a bigger sea.")
sleep(1)
print("You can also choose a harder difficulty.")
sleep(1)
input("Let's get started!\n(Press [Enter] to continue)")
sleep(1)
elif inp in no:
input(":(\n ):\nBut anyways...\nLet's get started!\n(Press" +
"[Enter] to continue) ")
else:
print("OK!")
sleep(1)
input("Let's get started!\n(Press [Enter] to continue) ")
sleep(1)
print("\nWhat would you like the board to look like?\nType anything f" +
'or the default look and type "2" or "3" for the other looks')
look = input("Look: ")
if look is "2":
print("OK! Look 2.\n")
elif look is "3":
print("OK! Look 3.\n")
else:
print('OK! Look "' + look + '", AKA the default.\n')

stillPlaying = True
while stillPlaying:
length = 0
height = 0
tries = 0
board = []

def dif2():
global difficulty
# Already tried once
tries = 1
while True:
print('Please enter "easy", "medium" or "hard"')
inp = input("Difficulty: ").lower()
if inp in ["0", "easy", "e", "beginner", "f

Solution

Naming conventions

Some of your variable names could be more descriptive.

For example you have the global variable names yes and no which don't definitively describe the purpose of those variable.
Anyone using the code would then need to look for where those variables were defined to understand what they were doing.
This is especially problematic because they are global variables so anyone who sees this in
a function now needs to look through all of the code to find the definition.

Another example is:

def boardGen(a):


What is a here? Without any docstring to describe your
parameter I have no indication as to what a is without looking through the rest of your
code to find out how you call the function.

Consider renaming the variable to something more descriptive and include a docstring that
explains what the parameter does, this will make life much easier for anyone reading your code.

Data structures

yes and no are collections of strings and the order does not matter.
So I would use a set for these:

yes_strings = {
    "yes",
    "y",
    "yeah",
}
no_strings = {
    "no",
    "n",
    "nope",
}


If you end up with a large number of strings this will help you look up elements faster.
It also more clearly states the intent of your code.

Magic numbers

Your code has a number of places where you use "magic numbers".
Essentially using unnamed numerical constants is a bad idea when the actual number is unimportant.

For example when you set the difficulty you have code like this:

if inp in ["0", "easy", "e", "beginner", "fácil"]:
    difficulty = 0


The difficulty is just a category and is not inherently any number.
I such cases I would make this a named variable like this:

DIFFICULTY_EASY = 0


Then use it like so:

if inp in ["0", "easy", "e", "beginner", "fácil"]:
    difficulty = DIFFICULTY_EASY


This makes the intent of the code much clearer and will help you avoid nasty mistakes
that can occur if the wrong number is used.

Later on when you are using the variable, compare this:

if difficulty == 4:
      #did I use the right number?


with this:

if difficulty == DIFFICULTY_HARD:
      #OK this is obviously correct


Program structure

The biggest issue with this code is the lack of structure.
Having too many global variables is usually a sign of problems with the program structure.
Essentially it makes it harder for you to maintain your program when you have to search
throughout all the code in order to find what can change different variables.
On top of that people can change those variables from anywhere and this makes it much
harder to reason about the state of your program.

This type of game is highly amenable to a class that has various
members to keep track of the state and methods to manipulate that state.
This way all of your game state variables are kept together and this makes maintenance a lot easier.

class GameState():
    """Stores the state of an individual game"""
    def __init__(self):
        self.difficulty = 0
        self.board_length = 0
        self.board_height = 0
        self.board = []


Now the state of the game is a lot more contained as it can all be found in this class.
Instead of the free functions that were manipulating the state you would now make those methods:

class GameState():
    """Stores the state of an individual game"""
    easy_difficulty_strings = {"0", "easy", "e", "beginner", "fácil"}
    medium_difficulty_strings = {"1", "medium", "m", "amateur", "médio"}
    hard_difficulty_strings = {"2", "hard", "h", "expert", "professional", "difícil"}

    def set_difficulty(self):
        """Set the difficulty for this game"""
        inp = input("Difficulty::    ").lower()
        if inp in easy_difficulty_strings:
            self.difficulty = DIFFICULTY_EASY
        elif inp in medium_difficulty_strings:
            self.difficulty = DIFFICULTY_MEDIUM
        elif inp in medium_difficulty_strings:
            self.difficulty = DIFFICULTY_HARD


Now the set_difficulty is a method of the GameState class and we have more structure.

This code can now be used like so:

current_game_state = GameState()
current_game_state.set_difficulty()


The main point is that in order to find anything to do with the state of the current game
don't need to look any further than current_game_state.

Program entry point

As it stands there's no clear entry point for this script.
There's some input handling code at the top of the file and then some
functions followed by more interaction with the user.
Keeping all this code together will help the clarity of your code.
The best way to do this is to write a main function as this makes perfectly
clear where the execution of the program starts:

def main():
    """Entry point for the program"""
    #All the code that was not in a function now goes here

if __name__ == "__main__":
    main()


Separate game logic and UI code

Code Snippets

def boardGen(a):
yes_strings = {
    "yes",
    "y",
    "yeah",
}
no_strings = {
    "no",
    "n",
    "nope",
}
if inp in ["0", "easy", "e", "beginner", "fácil"]:
    difficulty = 0
DIFFICULTY_EASY = 0
if inp in ["0", "easy", "e", "beginner", "fácil"]:
    difficulty = DIFFICULTY_EASY

Context

StackExchange Code Review Q#73603, answer score: 8

Revisions (0)

No revisions yet.