Battleships game in Python

Your program is pretty good, imports at the top, some functions and comprehensions. But they can all be improved.

• To adhere to PEP8 you should only import one module per import.
• You need some more functions, the ones your are using I would not use and would infuriate me.

• This is more an unwritten notion, but _ is known as the throw away variable. Since you throw away the range's items you could do:

  [' ' for _ in range(gridSize)]
  

I'd change the way you display the rows, currently you display two characters per print, this is quite hard to read, and not very efficient. Instead I'd recommend a list comprehension!

You display a space if getGrid(x,y) is "C", otherwise getGrid(x,y), for each character in that row. So:

row = [" " if getGrid(x, y) == "C" else getGrid(x, y) for y in range(gridSize)]

To then display this row you should use ' '.join(row). This puts a single space in-between each position on the board.

The other major change I'd make is the use of str.format. Take the following line:

print(" " + result + " [Ships: " + str(ships) + ", Size: " + str(gridSize) + ", Hits: " + str(hits) + "]\n")

That's hard to read with all the string concatenations and conversion, which str.format tidies up. The equivalent would be:

print(" {} [Ships: {}, Size: {}, Hits: {}]\n".format(result, ships, gridSize, hits))

You can now tell the format of the string, and can reasonably guess what will go there. All in little to no time.

For a few additional changes I'd recommend you change range with enumerate to get the x positions and the row, where you display the grid. And I'd completely remove the range when using the row comprehension. To show this:

for x, row in enumerate(game):
    print(' {} {}'.format(x, ' '.join([" " if y == "C" else y for y in row])))

I would also change the boards variable name to board so that people can instantly understand what you mean when you reference it.

In Python you should use snake_case not pascalCase for functions and variables, so gridSize should be grid_size.

And finally, you should use a try-except block when you are converting user input to a number as CAD97 put in their answer. This is as your program will crash if I enter "a".

To improve the xLabel creation you can think of it like we did the row, however you display the item you get from the range. This means that the list comprehension that you'd do is:

row = [str(i) for i in range(gridSize)]
xLabel = " " * 3 + " ".join(row)

First, let me say that I love the way you've done your interface. It's very nice and neat for a command-line program. User-wise, this is a nice design.

But let's get on to the potential improvements.

First, a couple things about input.

 Make a move! [Ships: 10, Size: 10, Hits: 0]

   0 1 2 3 4 5 6 7 8 9
 0
 1
 2
 3                 M
 4
 5
 6
 7
 8
 9

 X: 3
 Y: 8

You've mixed up your X and your Y coordinates. I've specified to go to (3,8) -- 3 units over and 8 units down -- and you've gone to (8,3) -- 8 units over and 3 units down. This is just a simple matter of realizing that your 2D List is in row-major order, and you have to index it as game[y][x] instead of game[x][y].

A 2D list is nothing special; it is just a list of list, and as such it has a structure like this:

[0:[0:'a', 1:'b', 2:'c'],
 1:[0:'d', 1:'e', 2:'f'],
 2:[0:'g', 1:'h', 2:'i']]

so to access an element you have to specify the row first (y) and then the column (x). As SuperBiasedMan said, you can do this easily just by using your getGrid and setGrid.

Secondly for input, this is something that we would prefer not to see: ValueError: invalid literal for int(). As your program is currently laid out, providing invalid input, such as the empty string or a non-number, will crash the program. If at all possible (and it is), you want to handle bad input gracefully. I will cover this in a bit.

So, on to talking about the code itself. After all, this is Code Review.

In Python, though you can place code at the lowest indent level to have it run, it is best to put it in a function. To do so, you follow this general outline:

import random, os

# your functions

def main():
    # your code

if __name__ == "__main__":
    main()

This encapsulates your logic into a main function and calls it when you run this script. Check This StackOverflow question for a great explanation of if __name__ == "__main__". Basically, it tells Python to only run this code when the script is run as the main task.

Now that we're defining functions, let's take care of the input problem. The basic idea of a fail-safe input is to try to parse the input, catch any errors if they exist, then try again if we failed. So, input_int could look something like this:

def input_int(prompt):
    while True:
        try:
            return int(input(prompt))
        except ValueError:
            print('Bad input; try again.')

Then, instead of wherever you call int(input("some string")) you use input_int("some string").

Joe Wallis has great advice on how to format your strings, so I will skip over covering that.

After that the only thing left to mention are magic constants. Rather than reference 'C', 'H', and 'M' directly in your code, define names for these values. Python doesn't have an explicit constant declaration, but the standard is SHOUTY_CASE for constants.

HIT = 'H'
MISS = 'M'
COMPUTER = 'C'
NONE = ' '

Then it will be easy to change these characters should you ever want to.

And again, Pep 8 is the official Python style guide. If you want to write code others can understand, I would highly suggest giving it a read.

I love little command line games like this-- reminds me of summer nights with the C64. The only thing that bugs me is that you're randomly choosing a new grid location, but you're not checking to see if it's already occupied.

Besides the logistical difficulties in parking one battleship atop another, if random.randint picks two duplicate x, y locations, you'll never achieve an end-game condition.

In my mind, this is the perfect opportunity for simple recursion! Recursion is fun, because it's slightly mindbending (at first), and never fails to impress the ladies. (Full disclosure: my wife is not impressed by recursion.)

I'd break out the 'random.randint' assignments to a separate function and do something like this:

def picker():
    x = random.randint(0,gridSize-1)
    y = random.randint(0,gridSize-1)
    if getGrid(x, y) == 'C':
        x, y = picker()
    return x, y

It checks to see if that location is in use. If so, it just calls itself, again (and again, possibly), and returns the final result. Then, change your ship placement loop to this:

for i in range(ships):              
    x, y = picker()
    setGrid(x,y,'C')

Of course, if you decide to place 101+ ships, it'll run until it kicks out a maximum recursion depth error. (which won't take long)

I don't think you need getGrid or setGrid (at least not in their current form). getGrid(x, y) is actually just less clear and longer than game[x][y]. People are familiar with what the indexing syntax means, but a function call could be doing anything.

On that note, why not keep getGrid so it can do something. You never check if the player has entered valid inputs. You just pass them directly. You could easily add a try except to catch index errors to make getGrid a valuable function:

def getGrid(x, y):
    try:
        return game[x][y]
    except IndexError:
        return None

This means that instead of having ugly program breaking errors you can return no result from this function.