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()

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!

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)