# 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

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 isDefeated(self,side):
for row in self.grid:
for piece in row:
if piece == side:
return False
return True

# Display the grid for the user

def show(self):

print("-" * 45)
line = ""
for i in range(10):
line += "| " + str(i) + " "
print(line + "|   |")
print("-" * 45)

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 "

line += "| " + str(y) + " |\n"

for x in self.grid[y]:
line+="----"
line += "-----"
print(line)

# Returns if a grid is empty

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

# Returns the value inside a coord

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

# Class which handles the battleships game

class Battleships:

# Allow the user to choose from 3 options

def __init__(self):
while True:
print("Welcome to Battleships!")
print("You can - ")
print("[1] PvP\n[2] PvE\n[3]Just Shoot Ships")
decision = 0

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

if decision == 1:
self.PVP()
elif decision == 2:
self.PVE()
elif decision == 3:
self.JSS()

orientation = 0
while orientation < 1 or orientation > 2:
try:
orientation = int(input("Would you like the ship to be horizontal [1] or vertical [2]: "))
except:
orientation = 0

if orientation == 1:
rootCoord = self.inputCoord(10-(length),9)
elif orientation == 2:
rootCoord = self.inputCoord(9,10-(length))

ship = []

# Whilst valid ship has not been added

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!")

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

if validShip:
ship = currentShip
return ship

# Function returns list of ship lengths that has been sunk

def getSunkShips(self,grid,side):

# List of sunk ships

sunkShips = []

# Go through the pieces array in grid object
for ship in range(1,len(grid.pieces[side])):
sunkStatus = []

# For each ship coordinate in a ship

for shipCoord in grid.pieces[side][ship]:

# If the coordinate can be found in the hit list then that part has been sunk

sunk = False
for hitCoord in range(1,len(grid.pieces[4])):
if shipCoord == grid.pieces[4][hitCoord][0]:
sunk = True
break
sunkStatus.append(sunk)

# Go through the sunk parts and if all of it is sunk then the ship is sunk

sunkShip = True
for status in sunkStatus:
if status == False:
sunkShip = False
break

if sunkShip == True:
sunkShips.append(ship+1)
return sunkShips

# Method for when the user wants to play against the computer

def PVE(self):

# Create grids

grids = [Grid(10,10),Grid(10,10)]

# Add ships for player 1

print("You input the root coordinate of the ship. They extend to the right or down, depending on orientation.")

# Add ships of length 2 - 6

for shipLength in range(5):
input("Press enter to continue...")
self.clearScreen()
print("Okay, the grids are set!")
self.clearScreen()

grids[1].grid = self.makeShips(grids[1],0,[1,1,1,1,1])

turn = 1

# Lists of coords the computer should shoot next

compWaitList = [[]]

# Coords the computer has tried
compShotList = []
compSunkShips = []
compPreviousHits = []

# While there are ships on both side

while grids[0].isDefeated(1) == False and grids[1].isDefeated(0) == False:

# If it is player 1's turn

if turn == 1:

print("Player 1's turn to shoot.")
grids[1].show()
validMove = False

while validMove == False:

# Get shot input and try to shoot
# If shot is not invalid then update the grid

shot = self.inputCoord(9,9)
potentialGrid = self.shoot(grids[1],0,shot)

if potentialGrid != "invalid":
grids[1].grid = potentialGrid
validMove = True
else:
continue

input("Press enter to continue.")
self.clearScreen()
print("Current grid for Player 1.")
grids[1].show()

# Check game is won or not

if grids[1].isDefeated(0) == True:
self.clearScreen()
print("Player 1 wins!")
input("Press enter to continue...")
self.clearScreen()
break

# If game is not won, tell the players of any full ships they have sunk.

self.sunkShips = self.getSunkShips(grids[1],0)
if len(self.sunkShips) >= 1:
print("Player 1 has sunk...")
for ship in self.sunkShips:
print("Ship of length {}.".format(ship))
else:
print("No ships have yet been sunk.")

input("Press enter for Computer's turn.")
self.clearScreen()
turn = 2

# Computer's turn

if turn == 2:

print("Computer's turn to shoot.")

validShot = False

# Get a possible x and y coordinate to shoot

while validShot == False:

x = -1
y = -1

if compWaitList == [[]]:
while x < 0 or x > 9:
x = random.randint(0,9)

while y < 0 or y > 9:
y = random.randint(0,9)

# Else take the first coord from the waiting list

else:
if compWaitList[0] != []:
x = compWaitList[0][0][0]
y = compWaitList[0][0][1]
compWaitList[0].pop(0)
else:
x = compWaitList[1][0][0]
y = compWaitList[1][0][1]
compWaitList[1].pop(0)

# If proposed x and y coordinate is in shot list then repeat loop
for coord in compShotList:
if coord == [x,y]:
break

validShot = True

print("Computer is deciding...")
time.sleep(1)

# Shoot with coords and also add them to used coords

compShotList.append([x,y])
potentialGrid = self.shoot(grids[0],1,[x,y],True)
print("Computer shot at [{},{}].".format(x,y))

# If it was a hit then try adding smart coords to wait list

if potentialGrid[1] == "hit":
print("The computer hit!")
grids[0].show()

# If there has been previous hit of importance and there are still possible hit locations
if compPreviousHits != [] and compWaitList != []:

# If the number of sunk ship increases, get the sunk length then remove the last n-1 possible perpendicular coords

if compSunkShips != self.getSunkShips(grids[0],1):
sunkShipLength = self.getSunkShips(grids[0],1)
print(compSunkShips)
print(sunkShipLength)
for length in compSunkShips:
if sunkShipLength[0] == length:
sunkShipLength.pop(0)

compSunkShips = self.getSunkShips(grids[0],1)
compWaitList[0] = []

# Move the previous hit to last, to be removed

compPreviousHits.append(compPreviousHits[0])
compPreviousHits.pop(0)
compPreviousHits.append([x,y])

del compWaitList[len(compWaitList)-sunkShipLength[0]:]

if compWaitList == []:
compWaitList.append([])
del compPreviousHits[len(compPreviousHits)-sunkShipLength[0]:]

# Else find relation of the two coords
else:

# Set limits to relating to whether they're on the edge and tets relation to last hit

if compPreviousHits[0][0] == x:

# Add next parallel coord (in relation to the hit and previosuly hit coord) to high priority, and perpendicular coords to lowest priority
# This is so there is a higher probability of another hit

if compPreviousHits[0][1] < y:
compWaitList += [[[[x+1,y],[x-1,y]]]]
if y != 9:
compWaitList[0] = [[[x,y+1]]] + compWaitList[0]
else:
compWaitList += [[[[x+1,y],[x-1,y]]]]
if y != 0:
compWaitList[0] = [[x,y-1]] + compWaitList[0]

elif compPreviousHits[0][1] == y:
if compPreviousHits[0][0] < x:
compWaitList += [[[x,y-1],[x,y+1]]]
if x != 9:
compWaitList[0] = [[x+1,y]] + compWaitList[0]
else:
compWaitList += [[[x,y-1],[x,y+1]]]
if x != 0:
compWaitList[0] = [[x-1,y]] + compWaitList[0]

compPreviousHits.append(compPreviousHits[0])
compPreviousHits.pop(0)
compPreviousHits = [[x,y]] + compPreviousHits
else:

# Add adjacent coords to the waiting list, depending on position on the grid
if x == 0:
if y == 0:
compWaitList[0] = [[x+1,y]]
compWaitList.append([[x,y+1]])
elif y == 9:
compWaitList[0] = [[x+1,y]]
compWaitList.append([[x,y-1]])
else:
compWaitList[0] = [[x+1,y]]
compWaitList.append([[x,y-1],[x,y+1]])
elif x == 9:
if y == 0:
compWaitList[0] = [[x-1,y]]
compWaitList.append([[x,y+1]])
elif y == 9:
compWaitList[0] = [[x-1,y]]
compWaitList.append([[x,y-1]])
else:
compWaitList[0] = [[x-1,y]]
compWaitList.append([[x,y-1],[x,y+1]])
elif y == 0:
compWaitList[0] = [[x-1,y]]
compWaitList.append([[x+1,y],[x,y+1]])
elif y == 9:
compWaitList[0] = [[x-1,y]]
compWaitList.append([[x+1,y],[x,y-1]])
else:
compWaitList[0] = [[x-1,y]]
compWaitList.append([[x+1,y],[x,y-1],[x,y+1]])

compPreviousHits.append([x,y])

grids[0].grid = potentialGrid[0]
validMove = True
else:
print("The computer missed!")
grids[0].show()

# Check game is won or not

if grids[0].isDefeated(1) == True:
self.clearScreen()
grids[0].show()
print("Player 2 wins!")
input("Press enter to continue...")
self.clearScreen()
break

self.sunkShips = self.getSunkShips(grids[0],1)
if len(self.sunkShips) >= 1:
print("Computer has sunk...")
for ship in self.sunkShips:
print("Ship of length {}.".format(ship))
else:
print("No ships have yet been sunk.")

input("Press enter for Player 1's turn.")
self.clearScreen()
turn = 1

return

# Function takes in a grid, the opposing side, and the coordinates for the shot

def shoot(self,grid,oSide,shot,isComputer=False):

# Get value in the coord to be shot
coordValue = grid.getCoordValue(shot[0],shot[1])

# If the opponent is the computer

if oSide == 0:

# If value is miss or hit, it is an invalid move

if coordValue == "miss":
print("You've already shot there! Was a miss!")
return "invalid"
elif coordValue == "hit":
print("You've already shot there! Was a hit!")
return "invalid"

# If blank, miss

elif coordValue == "":
print("Miss!")
return grid.grid

# If computer piece, hit

elif coordValue == 0:
print("Hit!")
return grid.grid

elif oSide == 1:
if isComputer == True:
if coordValue == "":
return [grid.grid,"miss"]
elif coordValue == 1:
return [grid.grid,"hit"]
else:
if coordValue == "miss":
print("You've already shot there! Was a miss!")
return "invalid"
elif coordValue == "hit":
print("You've already shot there! Was a hit!")
return "invalid"

# If shooting at side 2 (own), then it is invalid

elif coordValue == 2:
print("You cannot shoot your own ships!")
return "invalid"
elif coordValue == "":
print("Miss!")
return grid.grid

# If opponet is 1 and you shoot 1 then it is hit

elif coordValue == 1:
print("Hit!")
return grid.grid

elif oSide == 2:
if coordValue == "miss":
print("You've already shot there! Was a miss!")
return "invalid"
elif coordValue == "hit":
print("You've already shot there! Was a hit!")
return "invalid"

# If shooting at side 1 (own), then it is invalid

elif coordValue == 1:
print("You cannot shoot your own ships!")
return "invalid"
elif coordValue == "":
print("Miss!")
return grid.grid

# If opponet is 2 and you shoot 2 then it is hit

elif coordValue == 2:
print("Hit!")
return grid.grid

# Function takes in a grid, and the number of different ships wanted to add

def makeShips(self,grid,side,shipLengths):

# Add ships of varying lengths

for length in range(len(shipLengths)):

# Adds amount of ships for that length
for amount in range(shipLengths[length]):
ship = self.makeShip(grid,length+2)
return grid.grid

# Function returns array of coordinates for a ship

def makeShip(self,grid,length):
ship = []

# Randomise orientation

orientation = random.randint(1,2)

# Whilst valid ship has not been added

while True:
currentShip = []

# Get root position depending on orientation

if orientation == 1:
x = random.randint(0,10-length)
y = random.randint(0,9)
elif orientation == 2:
x = random.randint(0,9)
y = random.randint(0,10-length)

# Add coords depending on length

for i in range(length):
if orientation == 1:
currentShip.append([x+i,y])
elif orientation == 2:
currentShip.append([x,y+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

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

if validShip:
keepTrying = False
ship = currentShip
return ship

# Function takes in coordinate inputs

def inputCoord(self,maxX,maxY):
x = -1
y = -1

# While the coordinates are not within grid params

while x < 0 or x > maxX:
try:
x = int(input("Enter X coordinate: "))
except:
x = -1

while y < 0 or y > maxY:
try:
y = int(input("Enter Y coordinate: "))
except:
y = -1

return [x,y]

#Clears the console

def clearScreen(self):
print("\n" * 100)

game = Battleships()

• First, check out the pep-8 guidelines for python, and follow them. If you want incentive to do it right, and a helpful aide along the way, install PyCharm! Commented Nov 19, 2016 at 4:23

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 < 1 or 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 < start or 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!")

# 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!")

currentShip.append(attemptCoords)
return currentShip


There is no need for a while True. Before adding the coords to the currentShip, we can check if they are in an empty part of the grid. If they aren't we abort early. The boolean validShip doesn't actually do anything, if you reach that part of the code you know it has to be True. That means it can be removed

for shipCoord in grid.pieces[side][ship]:

# If the coordinate can be found in the hit list then that part has been sunk

sunk = False
for hitCoord in range(1,len(grid.pieces[4])):
if shipCoord == grid.pieces[4][hitCoord][0]:
sunk = True
break
sunkStatus.append(sunk)

# Go through the sunk parts and if all of it is sunk then the ship is sunk

sunkShip = True
for status in sunkStatus:
if status == False:
sunkShip = False
break

if sunkShip == True:
sunkShips.append(ship+1)


It is never necessary to have a condition with == True or == False. This is another good place for any and all to appear

for shipCoord in grid.pieces[side][ship]:
# If the coordinate can be found in the hit list then that part has been sunk

hitShips = grid.pieces[4]
sunk = any(shipCoord == hitShips[hitCoord][0] for hitCoord in range(1, len(hitShips)))
sunkStatus.append(sunk)

# Go through the sunk parts and if all of it is sunk then the ship is sunk

sunkShip = all(sunkStatus)

if sunkShip:
sunkShips.append(ship+1)


I will admit the code right now is not the most readable, but I think that is down to variable names now, as opposed what is going on in the code.

The rest of the code looks kinda dense right now, I would suggest going through the above suggestions and any other replies you get, make some improvements and post again with the next iteration of code.

I hope to see you post an update soon, all the best!

• Hey, sorry for replying so late. I have been busy revising for practice exams and also working on a Graphs class. Thanks for the advice, it is really appreciated. The .py files are currently at school and I'm on a break, once I get back I will try to implement these changes when I get the spare time. You have no idea how valuable this advice really is, thank you so much for taking the time to do this! Commented Dec 26, 2016 at 17:08

I mentioned some points, though you didn't explicitly asked for it. So please excuse me for ignoring this a little bit (I know that's unpolite).

• The use of lists:
your list self.pieces contains lists of numbers and lists of strings. It seems like you mix different usages in one variable. I would try to separate them if it is possible.
• clarity:
you have much code. So try to prevent unnecessary empty lines (of course not every empty line, only those which don't help to make the code readable) and in your case I would consider about programming some parts of this code in different modules.
• modularity (in a different way):
maybe you want to use some parts of your code again (in other projects). However, if you would import your current code, you would start the whole game automatically. Instead I would generate the Battleships-Object only if you run this script as main. You can check this with the following if-statement:

if __name__ == '__main__':
game = Battleships()


Of course, this has more to do with principles than with real usage in this case.

• commenting:
before you define a function, you explain the use of the function in a comment. That's actually good, but it is better to comment in the function and to use multi-line comments. A good editor/IDE recognizes the multi-line comments and shows it at tooltip when your cursor hovers over a function-call somewhere else in your code. For example I would change

#Clears the console

def clearScreen(self):
print("\n" * 100)


to

def clearCreen(self):
"""
Clears the console
"""
print("\n" * 100)

• Thank you for the reply! I've been busy with school and working on another project that I haven't been checking back on this. Advice and suggestions like these are so valuable, I really do appreciate it. I haven't looked into the name and "main" concept, so I will when I get back to school and have access to the files. Thanks so much! Commented Dec 26, 2016 at 17:10