I modified my Battleship game. Please critique!


I made the following modifications to my Battleship game during the lesson:

  • Made the ship 3 spaces long, randomly selecting a vertical/horizontal orientation
  • Added a function to draw the ship on the map with "#" characters at the end of the game
  • Added markers for which of the ship's 3 spaces was hit ("@") in the event of a win
  • Set user input to values 1-5 instead of confusing zero-index 0-4
  • Added an error message for non-integer input (goes to the existing "Oops" message rather than throwing a console error)
  • Put the whole game inside a function so I can call it again with a "reset" function which I run at the end of the game

I am TOTALLY new to coding and this was just my attempt to see what I could do on my own! I'm sure it's hideous and I break all kinds of style rules! Please let me know how I'm doing! Thanks!

from random import randint

#draw board
board = []

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

def print_board(board):
    for row in board:
        print " ".join(row)

#set ship origin
def random_row(board):
    return randint(0, len(board) - 1)

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

ship_row1 = random_row(board)
ship_col1 = random_col(board)

#set ship orientation
orientation = randint(0,100)
if orientation > 50:
    orientation = "v"
    orientation = "h"

#draw vertical ship
if orientation == "v":
    ship_row2 = ship_row1 + 1
    if (ship_row2 > 4):
        ship_row2 = ship_row1 - 1
    ship_row3 = ship_row2 + 1
    if (ship_row3 > 4):
        ship_row3 = ship_row1 - 1
    ship_col2 = ship_col1
    ship_col3 = ship_col1
    #this draws the ship on the map at the end of the game
    def print_ship():
        board[ship_row1][ship_col1] = "#"
        board[ship_row2][ship_col1] = "#"
        board[ship_row3][ship_col1] = "#"

#draw horizontal ship
if orientation == "h":
    ship_col2 = ship_col1 + 1
    if (ship_col2 > 4):
        ship_col2 = ship_col1 - 1
    ship_col3 = ship_col2 + 1
    if (ship_col3 > 4):
        ship_col3 = ship_col1 - 1
    ship_row2 = ship_row1
    ship_row3 = ship_row1
    def print_ship():
        board[ship_row1][ship_col1] = "#"
        board[ship_row1][ship_col2] = "#"
        board[ship_row1][ship_col3] = "#"

#reset function to run at end of game
def reset():
    reset = raw_input("Play again? Y/N")
    if reset == "Y":
        print "Thanks for playing!"

print "Let's play Battleship!"
print "The ship is 3 units long, but it might be hanging over one of the edges."
print "A single hit to my battleship will sink it!!"
print "You have 4 guesses."
#loop through 4 times

#put game inside a function so I can run reset() at the end
def game():
    for turn in range(4):
        print "Turn", turn + 1
        guess_row = raw_input("Guess Row:")
        #backdoor for debugging
        if guess_row == "esc":
        #check guess row and col for integer values
        if guess_row.isdigit() == True:
            guess_row = int(guess_row) - 1
        guess_col = raw_input("Guess Col:")
        if guess_col.isdigit() == True:
            guess_col = int(guess_col) - 1
        #check if guess matches ship location
        if ((guess_row == ship_row1) or \
        (guess_row == ship_row2) or \
        (guess_row == ship_row3)) and \
        ((guess_col == ship_col1) or \
        (guess_col == ship_col2) or \
        (guess_col == ship_col3)):
            #draw the point where the ship was hit
            board[guess_row][guess_col] = "@"
            print "Congratulations! You sunk my battleship!"
            print "# = Battleship Location"
            print "@ = Your Hit"
            #error if not on map
            if (guess_row < 0 or guess_row > 4) or \
            (guess_col < 0 or guess_col > 4):
                print "Oops, that's not even in the ocean."
           #error if already guessed
            elif(board[guess_row][guess_col] == "X"):
                print "You guessed that one already."
            #generic miss 
                print "You missed my battleship!"
                #mark missed location
                board[guess_row][guess_col] = "X"
            #end after 4th turn
            if turn == 3:
                print "Game Over"
                #draw ship & board

#let ‘er rip!


Testing if a boolean is true is redundant, you can add as many == True as you want to the end of that, makes no difference.

escaping the linebreak is not required there

To make it more extensible, (would want to learn about classe first)
I would represent a ship as an object that contains information about whether it's vertical or horizontal, its length, starting position, and number of remaining intact units. Then I would make all the locations that are occupied by it refer to that object. Print functions would easily be able to determine how to print that and adding more ships would just be a matter of creating more such objects and making the board refer to them. Determining when the player wins can be done by keeping count of how many ships are still not fully destroyed.
Setting up number of ships and board size could be done with constants at the top of the file, or with user input, or randomly.
I would probably also look to keep game logic and user interface separate, using a class and creating game instances that keep track of the game state would be suitable.

You're doing quite alright with code style, there are a few things that are a bit off. What you could do about it is to run a style checker on it, there are a few online ones I think, I'd run them from the command line, silly windows users would probably run such tools from some bulky IDE. You should take whatever these tools say as suggestions, you decide yourself what you want to fix - they're a guide/help you find things.


This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.