Battleships Player vs Computer Algorithm

I am a Year 12 student studying CS for my A-Levels. I have had previous Python experience but it is obviously not to professional/industry standard. Therefore, my goal for this review is for my code to be criticised, allowing me to improve towards reaching such standards.

The program is a computer version of the Battleships game. I have taken out the PvP and "Just Shoot Ships" methods. I believe the most interesting part is the PVE() function. This is whether the logic is implemented for the computer to play against the player.

Here is what I think I'd like a review on:

• The use of variables and their names

• The use of classes - how I've passed objects into functions; I think my use of classes and passing in objects is fairly inconsistent. I would like to know if there is a way I could improve this.

• The use of list and whether it is good practice to use jagged arrays in such a way.

• The efficiency of my algorithm in general if possible.

• The effectiveness of commenting and how it can be improved to be more useful in a team environment

```
# Battleships 2K16
# Completed 11/11/16, 13:00
# Developed by Tommy Kong

# This is a Battleships game.
# It allows players to:
# Play against each other
# Play against a computer
# Practice shooting ships

import random, time

# Class for creating a grid

class Grid:

# Create gird of width and height

def __init__(self,width,height):
self.grid = []
self.pieces = [[0],[1],[2],["miss"],["hit"]]
for y in range(height):
row = []
for x in range(width):
row.append("")
self.grid.append(row)

# Add piece to given coordinates

def addPiece(self,piece,side):
for pieceSet in self.pieces:
if pieceSet[0] == side:
pieceSet.append(piece)

for coord in piece:
self.grid[coord[1]][coord[0]] = side

# Function checks if the grid still has pieces of a certain side

def isDefea

I'll mention some small things to make the code look a little more

def __init__(self,width,height):
    self.grid = []
    self.pieces = [[0],[1],[2],["miss"],["hit"]]
    for y in range(height):
        row = []
        for x in range(width):
            row.append("")
        self.grid.append(row)

You can write the inner loop with a list comprehension

row = ["" for x in range(width)]

In addition since self.pieces are never changed from instance to instance, you can move them out of the init

self.pieces = [[0],[1],[2],["miss"],["hit"]]

def __init__(self,width,height):
    self.grid = []
    for y in range(height):
        row = ["" for _ in range(width)]
        self.grid.append(row)

for pieceSet in self.pieces:
    if pieceSet[0] == side:
        pieceSet.append(piece)

This is equivalent to a list comprehension (or a filter) that might be easier to read. also reusing peiceSet for both the iterating variable and the list seems weird and possibly buggy

pieceSet = [piece for peice in self.pieces if pieceSet[0] == side]

for row in self.grid:
    for piece in row:
        if piece == side:
            return False
return True

This is a good case for the any function

for row in self.grid:
    if any(piece == side for piece in row):
        return False
return True

You could reduce this to a single any with two loops inside it, but it gets a bit long and unwieldy.

for y in range(len(self.grid)):
    line = ""
    for x in self.grid[y]:
        if x == "":
            line += "| - "
        elif x == 0:
            line += "| - "
        elif x == 1:
            line += "| - "
        elif x == 2:
            line += "| - "
        elif x == "miss":
            line += "| X "
        elif x == "hit":
            line += "| O "

The only place y is used is as an index. To quote Raymond Hettinger "There must be a better way". We can use enumerate to keep y, but make the iteration look a bit nicer

The conditions can be shortened a little too, though not everyone would say it is an improvement.

for y, row in enumerate(self.grid):
    line = ""
     for cell in row:
         if cell in ("", 0, 1, 2):
            line += "| - "
         elif cell == "miss":
             line += "| X "
         elif cell == "hit":
             line += "| O "
...

def isEmpty(self,x,y):
    if self.grid[y][x] == "":
        return True
    else:
        return False

Ahhhhhhhhhhhh, I really dislike this as there exists a much nicer (and more intuitive way)

def isEmpty(self, x, y):
    return self.grid[y][x] == ""

def getCoordValue(self,x,y):
    return self.grid[y][x]

I'm not sure this is useful, anywhere it is used could just directly index the array

while decision  3:
    try:
        decision = int(input("What would you like to do: "))
    except:
        decision = 0

This seems like a pretty good candidate for a method.

def getValueInRange(start, end, message=""):
    """Get a value from a user that must be bound to a range
    start and end are exclusive from this range"""
    choice = start  # something outside the range
    while choice  end:
        try:
            choice = int(input(message))
        except ValueError:
            pass
    return choice

There is no need to overwrite choice with the default again and again if it fails. Also a more explicit error makes debugging and reading easier. Finally I only catch on ValueError as something unexpected would be better off thrown back up rather than silently dying here.

while True:
    currentShip = []
    # Add coords depending on length

    for i in range(length):
        if orientation == 1:
            currentShip.append([rootCoord[0]+i,rootCoord[1]])
        elif orientation == 2:
            currentShip.append([rootCoord[0],rootCoord[1]+i])

    # Test that the coords are not filled already
    validShip = True
    for coord in currentShip:
        if grid.isEmpty(coord[0],coord[1]) == False:
            # If any coords are filled then the ship is invalid

            validShip = False
            print("There are already ships existing there!")
            return self.addShip(grid,side,length)

    # If ship is valid then stop trying and return ship coords

    if validShip:
       ship = currentShip
       return ship

There is a lot going on here, so I'll show the improvements I would make an explain them after

currentShip = []
# Add coords depending on length

for i in range(length):
    if orientation == 1:
        attemptCoords = [rootCoord[0]+i, rootCoord[1]]
    elif orientation == 2:
        attemptCoords = [rootCoord[0], rootCoord[1]+i]

    if not grid.isEmpty(attemptCoords[0], attemptCoords[1]):
        print("There are already ships existing there!")
        return self.addShip(grid, side, length)

    currentShip.append(attemptCoords)
return currentShip

There is no need for a while True.
Before adding the

StackExchange Code Review Q#147478, answer score: 2