Python battleship program (2 player or 1 player)

Question

I recently started studying python with a much heavier study regimen, I wanted someone to review my code to see what they thought. I do have a to do list at the start of the code so any tips on how I could complete those would be appreciated.

Also, how are my comments? I'm really new to this whole code annotation thing so any feedback on that would be appreciated.

#ToDo list:
# 1: Multiple battleships, make sure to keep them on the board and dont let
# them overlap
# 2: Make battleships of different sizes, parts of battleships must be
# connected either vertically or horizontally. Also make sure to keep them
# on the board
# 3: Make the game 2 player
# 4: Give game more functionality with functions (pun intended), statistics,
# rematches, pretty much anything you can think of

from random import randint # Imports randint function

board = [] # Creates global board list

for x in range(5):
  board.append(["O"] * 5)
  # This function adds 5 "0"s per cycle, there are 5 cycles

def print_board(board):
  for row in board:
    print " ".join(row)
    # This function iterates through each "O" list and adds a space between
    # This iteration prints on a new line every time a new sub-list is found

def random_row(board):
  return randint(0, len(board) - 1)
  # This function selects a random list based on the number of sub-lists in the
  # Board list

def random_col(board):
  return randint(0, len(board[0]) - 1)
  # This function selects a random element in the first sub-list of the
  # Board list. This can be applied to every list, hence the column creation

ship_row = random_row(board) # Sets the row (x) position of the ship
ship_col = random_col(board) # Sets the column (y) position of the ship

def game():
    print_board(board) # Prints the board

    for turn in range(4): # Begins player V CPU cycle, does so 4 times
        guess_row = int(raw_input("Guess Row: ")) # Prompts user for row (x) guess, then converts to int
        guess_col = int(raw_input("Guess Col: ")) # Promps user for column (y) guess, then converts to int
        guess_row -= 1 # Brings guess down by one to account for CPU guess and board size
        guess_col -= 1 # "                                                              "

        if guess_row == ship_row and guess_col == ship_col: # Checks if player guess is same as randomly generated ship position
            print "Congratulations! You sunk my battleship!" # Prints out win message
            break # Cuts if statement and ends game

        else:
            if (guess_row < 0 or guess_row > 4) or (guess_col < 0 or guess_col > 4): # Checks if user guess is outside of board
                print "Oops, that's not even in the ocean." # Prints invalid pos message

            elif(board[guess_row][guess_col] == "X"): # Checks if user already guessed a position
                print "You guessed that one already." # Prints redundancy message

            else: # nested else statement covers if player missed the CPU ship
                print "You missed my battleship!" # Prints miss message
                board[guess_row][guess_col] = "X" # Sets board according to (incorrect) player guess_row
                print "---Turn", turn + 1, "---" # Prints turn number

                if turn == 3: # Checks to see if turn number is 3, then ends the game
                    print "Game Over"

            print_board(board) # Prints board if battleship is not sunk

def main():
    player = 0 # Sets player variable
    print "Welcome to battleship!" # Prints welcome message
    player = int(raw_input("Enter 1 for one player or 2 for two player:")) # Takes input to decide number of players

    if player == 1:
        game()
        # Starts single player game if only one player is selected
    elif player == 2:
        print "Welcome to the 2 player version of battleship, you will be facing head on with another human player!"
        print "Get ready to pass the comp!"
        print_board(board)
        print "Player one:"
        player_one_row_selection = int(raw_input("Please select a row:")) - 1
        player_one_col_selection = int(raw_input("Please select a column:")) - 1
        print "Player two:"
        # Prints 2 player welcome message, prints the board, and then promps player one for the battleship position

        for turn in range(4): # Starts turn loop
            print "---Turn", turn + 1, "---" # Prints turn every time
            player_two_row_guess = int(raw_input("Please select a row:")) - 1 # Takes input from player 2 for Turn n for row
            player_two_col_guess = int(raw_input("Please select a column:")) - 1 # Takes input from player 2 for Turn n for column

            if (player_two_row_guess == player_one_row_selection) and (player_two_col_guess == player_one_col_selection): # Checks to see if X coordinate guess and Y coordinate guess equals player one's selection
                board[player_two_row_guess][player_two_col_guess] = "X" # Sets an X at the point of selection
                print_board(board) # Prints the board
                print "Congratulations! You sunk their battleship!" # Prints victory message
                break # Exits if statement

            elif (player_two_row_guess < 0 or player_two_row_guess > 4) or (player_two_col_guess < 0 or player_two_col_guess > 4): # Checks to see if guess is out of bounds
                 print "Thats out of bounds!" # Prints out of bounds message

            elif board[player_two_row_guess][player_two_col_guess] == "X": # Checks to see if the player guessed a spot already
                print "You already guessed that!" # Prints appropriate message

            else:
                 print "You missed their battleship!" # If else, prints message that the battleship was missed
                 board[player_two_row_guess][player_two_col_guess] = "X" # Sets guess X Y coord to an X

                 if turn == 3: # Checks to see if turn 3 has been reached
                     print "Game Over" # Prints game over message

            print_board(board) # Prints board

    else: # If player enters something other than 1 or 2 it prints a message
        print "Please come back and select a valid number!" # prints "number invalid" message

main() # Starts game

Answer 1

1. Comments

Your comments aren't very good. But conveniently, your code is structured so as to make this critique easy:

from random import randint # Imports randint function

board = [] # Creates global board list

for x in range(5):
  board.append(["O"] * 5)
  # This function adds 5 "0"s per cycle, there are 5 cycles

def print_board(board):
  for row in board:
    print " ".join(row)
    # This function iterates through each "O" list and adds a space between
    # This iteration prints on a new line every time a new sub-list is found

Consider the first two comments: "Imports randint function" and "Creates global board list".

Those comments are garbage. Of course the from random import randint statement imports the randint function. It's defined to do exactly that! There is no value provided in your comment - it doesn't explain anything to anyone.

Get rid of those comments directly. Never write comments like that. It would be different if you were doing something like

import random  # contains randint function

At least that way it would be providing information. Your comments are not providing information, because the syntax of the language already specifies that information. An import statement imports. A variable created at the outermost module scope is a global.

Never insert a comment that explains language syntax, unless you are trying to understand someone else's code, in a language you are unfamiliar with.

Next, consider this: This function adds 5 "0"s per cycle, there are 5 cycles

This is useless to me, since I am familar with Python, but might actually be useful to you if you're just learning it, and/or if you got that snippet of code from a SO answer. The comment is inside the function, and discusses the behavior of the function. I would suggest moving the comment up, so it's at the top of the function. Otherwise, this is an acceptable comment (again: for someone new to Python, or unfamiliar with list comprehensions) in the right scope. I would grade this comment as a 'B'.

Finally, this comment: This function iterates through each "O" list and adds a space between. This iteration prints on a new line every time a new sub-list is found

This is useless again, because it duplicates the code. Also, it discusses things in an implementation-dependent manner that aren't obvious. I consider this to be useless since it doesn't say anything the code doesn't say, and it might get out of date if you change how things operate in a different function. Delete this, or rewrite it to explain what is truly unclear. (For example, if you are having trouble with how join works.)

2. Organization

I like the functions you have. I'd suggest that you write a setup_board or init_board function to handle the creation of the board. Likewise, something to position the ship, although that could be part of the setup function in this simple version. (If you do a huge battleship game with ships of different sizes, etc., you'll probably have separate functions. But that's later.)

Consider this code from main:

if player == 1:
    game()
    # Starts single player game if only one player is selected
elif player == 2:
    print "Welcome to the 2 player version of battleship, you will be facing head on with another human player!"
    print "Get ready to pass the comp!"

Why does a 1-player game have a function, but a 2-player game is in line? Either create a function that can handle both versions, or create two different functions. You want your code to be "balanced." You shouldn't zoom from details to highly abstract in a single function. So, if you are going to abstract the single-player game (yes!) then abstract the 2-player game as well.

Make your code DRY: Don't Repeat Yourself!

You say this a bunch of times: int(raw_input(

Maybe you need a function that reads in a number. Maybe it should take optional min/max parameters. Something like:

guess_row = read_int("Guess row: ", max=num_rows)
guess_col = read_int("Guess col: ", max=num_cols)

3. Logic

You have some pieces of code that could be cleaned up. For example:

for turn in range(4): # Begins player V CPU cycle, does so 4 times
    ...
    if turn == 3:
        print "Game Over"

You're inside a loop on range(4). There's no reason to check for game-over, because when the game is over the loop will exit and you will fall through to below the loop. Better to simply write:

for turn in range(4):
    ...

print "Game Over"

Let the loop do the work. (Note: your game-won code does a break. You could change that to a return, or you could use the for ... else: statement to catch the no-break case.

Also, you've got this structure:

print_board(board) # Prints the board

for turn in range(4): # Begins player V CPU cycle, does so 4 times 
    ... loop body ...
    print_board(board)

You can tighten that up by putting print_board at the top of the loop:

for turn in range(4):
    print_board(board)
    ... loop body ...

4. Idiom

There are two things that strike me as un-Pythonic. First, there's this:

main() # Starts game

There's a standard idiom for Python programs:

if __name__ == '__main__':
    main()

This makes it possible to write code that can be a stand-alone program, or a module. You should prefer it always, because that means you can write unit tests that load your code, execute a small part of it, and evaluate whether it works, without actually trying to run everything.

Note: you may not have learned about tuples yet, if you're doing this for a class. But since you have obviously learned about lists, I'm willing to suggest this just in case.

Second, you may not know that you can compare tuples for equality. So, a good way to compare 2 numbers with 2 other numbers, if you only care about total equality, is this:

if (guess_row, guess_col) == (ship_row, ship_col):
    print "Congratulations! You sunk my battleship!"

Of course, if you created a variable to store the ship position (at start of game), and a single variable to store both the guessed row and column:

ship_pos = (ship_row, ship_col)
guess = (guess_row, guess_col)

if guess == ship_pos:
    print "Congratulations! You sunk my battleship!" 

This is a lot easier to read, spell, and understand. Python loves tuples.

Answer 2

I wont extend much, I just have one thing for you. It is nice to comment and give explanations about what you are doing, really nice. You'll see that when you review your code 3-4 months in the future those comments will help you.

print_board(board) # Prints the board

This comment tho... give explanations about what you feel needs explanation, don't overdo it. You don't need to annotate every line, documentation is other thing.

def print_board(board):
    for row in board:
        print " ".join(row)
        # This function iterates through each "O" list and adds a space between
        # This iteration prints on a new line every time a new sub-list is found

The proper way of doing this is:

def print_board(board):
    """Takes board as argument, iterates through each "O" list and adds a space in between"""
    print ("\n".join([" ".join(row) for row in board]))  # This line need explanation maybe :S

This is called a documentation string, you can read some conventions in PEP257 (nothing will change in your life if you start reading PEPs btw) the good thing about this is that you can call help() on that function and that docstring will magically appear, i.e:

help(print_board)
Help on function print_board in module __main__:

print_board(board)
    Takes board as argument, iterates through each "O" list and adds a space in between

There are a lot of good things about docstrings, they let you build formated documentation, your IDE will look at them and provide you with some info, they are nice to you, be nice to them.

Hope it helps!