6
\$\begingroup\$

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ácil"]:
                difficulty = 0
                break
            elif inp in ["1", "medium", "m", "amateur", "médio"]:
                difficulty = 1
                break
            elif inp in ["2", "hard", "h", "expert", "professional",
                         "difícil"]:
                difficulty = 2
                break
            else:
                tries += 1
#               Some people just held the [enter] key.
                if tries is 10:
                    print("Enough with your shenanigans!")
                    print("I'm setting it to easy!")
                    print("Difficulty:   easy")
                    difficulty = 0
                    break

    def dif():
        global difficulty
        difficulty = ""
        print("What difficulty would you like to play on?\nEasy, medium",
              "or hard.")
        inp = input("Difficulty:   ").lower()
        if inp in ["0", "easy", "e", "beginner", "fácil"]:
            difficulty = 0
        elif inp in ["1", "medium", "m", "amateur", "médio"]:
            difficulty = 1
        elif inp in ["2", "hard", "h", "expert", "professional", "difícil"]:
            difficulty = 2
        else:
            dif2()

    def perm():
        """This is to prevent people to accidentally enter very large
        boards, as they aren't thet fun anyways and may use alot of
        memory.
        """
        global yes
        print("That board dimension you've picked is quite big.\nIt",
              "may make the game borderline unplayable.")
        if input("Are you sure you want a board this big? ").lower() in yes:
            print("I will take no responsibility for any damage done",
                  "to your console or PC.")
            if input("Are you sure? ") in yes:
                return True
        return False

    def boardGen(a):
        global length
        tries = 0
        while True:
            if a is "length":
                q = "Sea length:   "
            else:
                q = "Sea height:   "
            if tries is 10:
                print("OK, stop messing around!\nI'm setting it to 5!\n"+q+"5")
                return 5
            try:
                out = int(input(q))
            except ValueError:
                print("That's not a valid number! Try again!")
                tries += 1
                continue
            if isinstance(out, int):
                if out < 1:
                    print("That's too little! Try something 1 or more.")
                    tries += 1
                    continue
#               So you can't hava a 1 by 1 board and win
                elif out < 2 and length is 1:
                    print("Now that won't be any fun! Try something",
                          "2 or more.")
                    tries += 1
                    continue
                elif out > 49 and not perm():
                    tries += 1
                    continue
            break
        return out

    def turnGen():
        global area
#       area is how many squares in the board
        tries = 0
        while True:
            if tries is 10:
                print("Enough with your shenanigans!\nI'm setting it to 1!")
                print("Turns:        1")
                return 1
            try:
                out = int(input("Turns:        "))
            except ValueError:
                print("That's not a number! Try again!")
                tries += 1
                continue
            if isinstance(out, int):
                if out < 1:
                    print("That's too little! Try something 1 or more.")
                    tries += 1
                    continue
#               Area - 1 is so that you can't have as many turns as squares
#               so there's still that chance you lose.
                elif out > area-1:
                    print("That's no fun! Try something", area-1, "or under.")
                    tries += 1
                    continue
#               the 'and area > ' so you don't have to get
#               this message everytime you choose the smaller board sizes
                elif out > area / 2 and area > 5:
                    print("There won't be much strategy involved if you have",
                          "this many turns!")
                    if not (input("Are you sure? ") in yes):
                        tries += 1
                        continue
            return out

    def printBoard():
        global board, length, height, look
        if look is "2":
            count = 0
            mult = length * 2 + 1
            print("+" * mult)
            for l in board:
                print("+" + " ".join(l) + "+")
                count += 1
                if not count == height:
                    print("+" + " " * (mult-2) + "+")
            print("+" * mult)
        elif look is "3":
            for l in board:
                print("".join(l))
        else:
            for l in board:
                print(" ".join(l))

    def guessGen(a):
        tries = 0
        while True:
            if a is "row":
                q = "Guess row:    "
            else:
                q = "Guess column: "
            if tries == 9:
                print("You just need to type a number!")
            elif tries == 10:
                print("...\nI'm inputting 1 manually now.\n" + q + "1")
                return 1
            try:
                out = int(input(q))
            except ValueError:
                print("That's not a valid number! Try again!")
                tries += 1
                continue
#           out - 1 as list index starts at 0, not 1
            return out - 1

    def distance():
        global difficulty, shipRow, shipCol, guessRow, guessCol, area
        distRow = shipRow - guessRow
        distCol = shipCol - guessCol
        av = sqrt(area)
        s1 = ""
        s2 = ""
        if distRow < 0:
            s1 = "north "
#           To make it positive
            distRow *= -1
        elif distRow > 0:
            s1 = "south "
        if distCol < 0:
            s2 = "west "
            distCol *= -1
        elif distCol > 0:
            s2 = "east "
        dist = distCol + distRow
        if difficulty is 0:
            print("The battleship was to the " + s1 + s2 + "of that bomb.")
            print("It was also", dist, "squares away.")
        elif difficulty is 1:
            print("The battleship was", dist, "squares away.")
        elif difficulty is 2:
            if dist is 1:
                print("The battleship was right there!")
            elif dist < av / 16:
                print("The battleship was very close.")
            elif dist < av / 12:
                print("The battleship was close.")
            elif dist < av / 8:
                print("The battleship was pretty close.")
            elif dist < av / 4:
                print("The battleship was far")
            elif dist < av / 2:
                print("The battleship was very far")
            else:
                print("The battleship was nowhere near where the bomb landed.")
    won = False
    dif()
    length = boardGen("length")
    height = boardGen("height")
    area = height * length
    print("Length:      ", length, "\nHeight:      ", height)
    print("Area:        ", area)
    for i in range(height):
        board.append(["O"] * length)
    turns = turnGen()
    tries = 0
    print("Turns:       ", turns)
    shipRow = randint(0, len(board)-1)
    shipCol = randint(0, len(board[0])-1)
    for turn in range(turns):
        turnsRemaining = turns - turn
        print()
        if turn is 0:
            print("Let's play battleships!")
        elif turnsRemaining is 1:
            print("Last turn!")
        else:
            print(turnsRemaining, "turns remaining!")
        print("Turn", turn+1)
        printBoard()
        guessRow = guessGen("row")
        guessCol = guessGen("col")
        if guessRow is shipRow and guessCol is shipCol:
            print("Congratulations! You sunk the battleship!")
            wins += 1
            won = True
            break
        else:
            if guessRow < 0 or guessRow > height-1 or \
               guessCol < 0 or guessCol > length-1:
                print("Oops, that's not even in the ocean.")
                distance()
            elif board[guessRow][guessCol] is "X":
                print("You already guessed that one.")
                distance()
            else:
                print("You missed the battleship!")
                board[guessRow][guessCol] = "X"
                distance()
    board[shipRow][shipCol] = "B"
    print()
    printBoard()
    gamesPlayed += 1
    if won:
        s = "You won!"
    else:
        s = "Game over!"
    print(s, "The battleship (B) was in row", str(shipRow + 1) + ", column",
          str(shipCol + 1) + ".")
    inp = input("Play again? ")
    if inp in yes:
        continue
    elif inp in no:
        print(":(\n  ):")
        sleep(1)
        stillPlaying = False
    elif inp is "":
        print("This was put here so you couldn't just spam the",
              '"Enter" button repeatedly to play.')
        print("If this was a mistake, please don't press the",
              '"Enter" button without typing anything into this field.')
        stillPlaying = False
    else:
        print('I\'ll take that as a "Yes"')
        continue
if gamesPlayed is 1:
    s1 = "game"
else:
    s1 = "consecutive games"
if wins is 1:
    s2 = "You sunk one battleship"
else:
    s2 = "You sunk a total of " + str(wins) + " battleships"
if gamesPlayed is wins and s1 is "game":
    s3 = " and didn't let it get away!"
elif gamesPlayed is wins:
    s3 = " and didn't let any get away!"
elif gamesPlayed - wins is 1:
    s3 = ", but 1 got away!"
else:
    s3 = ", however, you let " + str(gamesPlayed-wins) + \
         " battleships get away!"
print("You played", gamesPlayed, s1, "of battleships!")
print(s2 + s3)
if (gamesPlayed-wins) * 2 > wins:
    print("Better luck next time!")
else:
    print("Well done!")
sleep(2)
input("(Press [enter] to terminate) ")

I would like feedback on making it shorter, because it is quite long for what seems like a simple game. I'd also like a review on the style of it (it passed all the PEP8 tests).

\$\endgroup\$
8
\$\begingroup\$

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

Currently the game logic and the UI code are completely intertwined. If at some time in the future you chose to change your program from the console based UI to a GUI or a website or something else then you would be facing a rewrite.

So for example the distance function I would try to remove all the prints and instead print results from the place that calls the distance function. This would involve returning a value from the distance function that represents the distance.

I notice that you have a variable look which appears to be used for determining the look of the program but it doesn't appear to be used. Even if it was used it would be hard to integrate it into the program as is because the UI and the game logic is not separated.

Next step

There's more that could be improved here but improving these basics first is the most important step you can take. If you improve the code don't hesitate to post a follow-up question including your improved code.

\$\endgroup\$
1
\$\begingroup\$

shuttle87 already gave overall answer so this is just practical example of how to make some parts of your program shorter.

I like to separate large amount of printable text from code. You can define function that creates text with predefined parameters and simply print result via other function.

def print_difficulty_dialog():
    text = ('\nPlease enter "easy", "medium" or "hard"',
            'Difficulty:   ')
    return text

def print_intro():
    text = ("A battle ship is hidden in a patch of ocean.",
            "You have some bombs to try to hit it.",
            "To deploy a bomb, we need to know where to launch it",
            "The most top left area is row 1, column 1.",
            "You can make it harder by having less turns and a bigger sea.",
            "You can also choose a harder difficulty.",
            "Let's get started!\n(Press [Enter] to continue)")
    return text

def print_warning():
    text = ('\nEnough with your shenanigans!',
            'I\'m setting it to easy!',
            'Difficulty:   easy')
    return text

def print_text(text_tuple, delay=0):
    for text in text_tuple:
        sleep(delay)
        print(text)
    word = input()
    return word

Note that print_text() not only prints text but can also return input() values if needed. And this code is easy to adapt for GUI program if needed.

Same with setting difficulty. Assuming that you will define GameState() class:

class GameState():
    ...
    def setDifficulty(self, word):
        legend_dict = {'easy': {"0", "easy", "e", "beginner"},
                       'medium': {"1", "medium", "m", "amateur"},
                       'hard': {"2", "hard", "h", "expert", "professional"}} 

        for difficulty in legend_dict.keys():
            if word in legend_dict[difficulty]:
                self.difficulty = difficulty
                break
        else: difficulty = 0
        return difficulty

Here I just save some time instead of writing all if/elif cases. Plus one can use different ways to input word in this method. shuttle87's version is more understandable though for small amount of if/elif statements.

And finally we can put all of this inside main loop in order to track how program is running:

def main():
    print_text(print_intro(), delay=1)

    tries = 0
    while True:
        word = print_text(print_difficulty_dialog()).lower()
        difficulty = GameState.setDifficulty(word)
        tries += 1
        if difficulty:
            break
        elif tries == 10:
            print_text(print_warning())
            break


if __name__ == '__main__':
    main()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy