Battleships Player vs Computer Algorithm

Question

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 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):
 # Adding the top coordinates
 print("-" * 45)
 line = ""
 for i in range(10):
 line += "| " + str(i) + " "
 print(line + "| |")
 print("-" * 45)
 # Adding the individual cells
 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 "
 # Adding side coordinates
 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()
 # Adds a ship
 def addShip(self,grid,side,length):
 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!")
 return self.addShip(grid,side,length)
 # 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)]
 print("Now you are going to add your ships.")
 # Add ships for player 1
 print("Player 1 add ships.")
 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):
 print("Add ship of length {}".format(shipLength+2))
 ship = self.addShip(grids[0],1,shipLength+2)
 grids[0].addPiece(ship,1)
 input("Press enter to continue...")
 self.clearScreen()
 print("Okay, the grids are set!")
 self.clearScreen()
 # Add ships for computer
 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)
 alreadyShot = False
 # If proposed x and y coordinate is in shot list then repeat loop
 for coord in compShotList:
 if coord == [x,y]:
 alreadyShot = True
 break
 if alreadyShot == False:
 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!")
 grid.addPiece([shot],"miss")
 return grid.grid
 # If computer piece, hit
 elif coordValue == 0:
 print("Hit!")
 grid.addPiece([shot],"hit")
 return grid.grid
 elif oSide == 1:
 if isComputer == True:
 if coordValue == "":
 grid.addPiece([shot],"miss")
 return [grid.grid,"miss"]
 elif coordValue == 1:
 grid.addPiece([shot],"hit")
 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!")
 grid.addPiece([shot],"miss")
 return grid.grid
 # If opponet is 1 and you shoot 1 then it is hit
 elif coordValue == 1:
 print("Hit!")
 grid.addPiece([shot],"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!")
 grid.addPiece([shot],"miss")
 return grid.grid
 # If opponet is 2 and you shoot 2 then it is hit
 elif coordValue == 2:
 print("Hit!")
 grid.addPiece([shot],"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)
 grid.addPiece(ship,side)
 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()

• Jake Dube: 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!

Answer 1

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

• Tommy Kong: 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!

Answer 2

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)
  

• Tommy Kong: 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!