Battleships OOP python

Question

This is my first attempt at some basic OOP programming. A version of battleships played within the terminal. Any feedback would be great; especially in regards to readability and proper naming conventions.

I would really like to improve the strike function of the computer class to make it play more intelligently, any tips on that specifically?

ocean.py

class Ocean:

    """Creates 2D array the represents an ocean grid
    to veiw your ship positions

    Class contains all functions needed for placing
    ships on the ocean grid"""

    def __init__(self, width=10, height=10):
        self.ocean = [["~" for i in range(width)] for i in range(height)]

    def __getitem__(self, point):
        row, col = point
        return self.ocean[row][col]

    def __setitem__(self, point, value):
        row, col = point
        self.ocean[row][col] = value

    def view_ocean(self):
        for row in self.ocean:
            print(" ".join(row))

    # Two functions check a coordinate input is on the grid

    def valid_col(self, row):
        try:
            self.ocean[row]
            return True
        except IndexError:
            return False

    def valid_row(self, col):
        try:
            self.ocean[0][col]
            return True
        except IndexError:
            return False

    # Two functions check for valid board space for ship placement

    def can_use_col(self, row, col, size):

        valid_coords = []

        for i in range(size):

            if self.valid_col(col) and self.valid_row(row):
                if self.ocean[row][col] == "~":
                    valid_coords.append((row, col))
                    col = col + 1
                else:
                    col = col + 1
            else:
                return False

        if size == len(valid_coords):
            return True
        else:
            return False

    def can_use_row(self, row, col, size):

        valid_coords = []

        for i in range(size):

            if self.valid_row(row) and self.valid_col(col):
                if self.ocean[row][col] == "~":
                    valid_coords.append((row, col))
                    row = row + 1
                else:
                    row = row + 1
            else:
                return False

        if size == len(valid_coords):
            return True
        else:
            return False

    # Corresponding fucntions set ship counters on valid space

    def set_ship_col(self, row, col, size):
        for i in range(size):
            self.ocean[row][col] = "S"
            col = col + 1

    def set_ship_row(self, row, col, size):
        for i in range(size):
            self.ocean[row][col] = "S"
            row = row + 1

radar.py

class Radar:

    """Creates a grid to track the state of an opponent's ocean grid"""

    def __init__(self, width=10, height=10):
        self.radar = [["." for i in range(width)] for i in range(height)]

    def __getitem__(self, point):
        row, col = point
        return self.radar[row][col]

    def __setitem__(self, point, value):
        row, col = point
        self.radar[row][col] = value

    def view_radar(self):
        for row in self.radar:
            print(" ".join(row))

ship.py

class Ship:

    def __init__(self, ship_type, size):
        self.ship_type = ship_type
        self.size = size
        self.coords = []

    def plot_vertical(self, row, col):
        for i in range(self.size):
            self.coords.append((row, col))
            row = row + 1

    def plot_horizontal(self, row, col):
        for i in range(self.size):
            self.coords.append((row, col))
            col = col + 1

    def check_status(self):
        if self.coords == []:
            return True
        else:
            return False

player.py

from ocean import Ocean
from radar import Radar
from ship import Ship
import os

class Player:

    ships = {"Aircraft Carrier": 5, "Crusier": 4, "Destroyer": 3, 
"Submarine": 2}

    def __init__(self, name):
        self.ocean = Ocean()
        self.radar = Radar()
        self.name = name
        self.fleet = []

   # Function uses player input to set up fleet positions on a player board.
   # For each ship, a ship object containing relevant coordinates is appended to self.fleet

    def set_fleet(self):
        input("Pick a coordinate between 0 and 9 for the columns and rows on your board")
        input("Boats are placed form right to left.")
        for ship, size in self.ships.items():

            flag = True
            while flag:
                self.view_console()
                try:
                    print("Place your %s" % (ship))
                    row = int(input("Pick a row -----> "))
                    col = int(input("Pick a column -----> "))
                    orientation = str(input("Vertical or Horizontal? (choose v or h) -----> "))

                    if orientation in ["v", "V"]:
                        if self.ocean.can_use_row(row, col, size):
                            self.ocean.set_ship_row(row, col, size)
                            boat = Ship(ship, size)
                            boat.plot_vertical(row, col)
                            self.fleet.append(boat)
                            flag = False
                        else:
                            input("Overlapping ships, try again")

                    elif orientation in ["h", "H"]:
                        if self.ocean.can_use_col(row, col, size):
                            self.ocean.set_ship_col(row, col, size)
                            boat = Ship(ship, size)
                            boat.plot_horizontal(row, col)
                            self.fleet.append(boat)
                            flag = False
                        else:
                            input("Overlapping ships, try agin")

                    else:
                        continue

                    self.view_console()
                    input()
                    os.system('clear')

                except ValueError:
                    print("Don't you remember your training?\nType a number..\n")

    # Function displays player ocean/radar in readable format

    def view_console(self):
        self.radar.view_radar()
        print("|                 |")
        self.ocean.view_ocean()

    # Function checks status of ship objects within player fleet

    def register_hit(self, row, col):
        for boat in self.fleet:
            if (row, col) in boat.coords:
                boat.coords.remove((row, col))
                if boat.check_status():
                    self.fleet.remove(boat)
                    print("%s's %s has been sunk!" % (self.name, boat.ship_type))

    # Player interface for initiating in-game strikes,
    # updates the state of the boards of both players

    def strike(self, target):
        self.view_console()
        try:
            print("\n%s Pick your target..." % (self.name))
            row = int(input("Pick a row -----> "))
            col = int(input("Pick a column -----> "))

            if self.ocean.valid_row(row) and self.ocean.valid_col(col):
                if target.ocean.ocean[row][col] == "S":
                    print("DIRECT HIT!!!")
                    target.ocean.ocean[row][col] = "X"
                    target.register_hit(row, col)
                    self.radar.radar[row][col] = "X"

                else:
                    if self.radar.radar[row][col] == "O":
                        print("Area already hit....Check your radar!")
                        self.strike(target)
                    else:
                        print("Negative...")
                        self.radar.radar[row][col] = "O"

            else:
                print("Coordinates out of range...")
                self.strike(target)

        except ValueError:
            print("You need to provide a number....\n")
            self.strike(target)
        input()
        os.system('clear')

computer.py

from player import Player
from ship import Ship
import random


class Computer(Player):

    def __init__(self):
        super().__init__(self)
        self.name = "Computer"

    # Automated version of set_fleet function from Player

    def set_compu_fleet(self):
        positions = ["v", "h"]

        for ship, size in self.ships.items():

            flag = True
            while flag:
                row = random.randint(0, 9)
                col = random.randint(0, 9)
                orientation = random.choice(positions)

                if orientation == "v":
                    if self.ocean.can_use_row(row, col, size):
                        self.ocean.set_ship_row(row, col, size)
                        boat = Ship(ship, size)
                        boat.plot_vertical(row, col)
                        self.fleet.append(boat)
                        flag = False

                    else:
                        row = row + 2

                elif orientation == "h":
                    if self.ocean.can_use_col(row, col, size):
                        self.ocean.set_ship_col(row, col, size)
                        boat = Ship(ship, size)
                        boat.plot_horizontal(row, col)
                        self.fleet.append(boat)
                        flag = False

                    else:
                        col = col + 2

                else:
                    continue

    # Automated strike function

    def compu_strike(self, target):
        row = random.randint(0, 9)
        col = random.randint(0, 9)

        if self.radar.radar[row][col] == ".":
            input("...Target acquired....%s, %s" % (row, col))

            if target.ocean.ocean[row][col] == "S":
                print("DIRECT HIT!")
                target.ocean.ocean[row][col] = "X"
                target.register_hit(row, col)
                self.radar.radar[row][col] = "X"

            else:
                print("Missed....recalibrating")
                self.radar.radar[row][col] = "O"

        else:
            self.compu_strike(target)

battlshipspvp.py

from player import Player
import os

# Player versus player mode


class BattleshipsPVP:
    """creates a game of battlehsips"""

    def __init__(self):
        start = input("Begin? (y or n) -----> ")
        if start in ["y", "Y"]:
            self.playPVP()
        else:
            print("Aborted...")

    def playPVP(self):
        p1name = input("Player 1, state your name! -----> ")
        p1 = Player(p1name)
        p1.set_fleet()
        p1.view_console()
        self.clear_screen()

        p2name = input("\n\nPlayer 2, state your name! -----> ") 
        p2 = Player(p2name)
        p2.set_fleet()
        p2.view_console()
        self.clear_screen()

        flag = True
        while flag is True:
            p1.strike(p2)
            if self.fleet_sunk(p2) is True:
                self.victory_message(p1, p2)
                flag = False
            else:
                self.clear_screen()

                p2.strike(p1)
                if self.fleet_sunk(p1) is True:
                    self.victory_message(p2, p1)
                    flag = False
                else:
                    self.clear_screen()
        print("\nThanks for playing!")

    # Function checks remaining ship counters on a player's board

    def fleet_sunk(self, player):
        ship_counters = 0
        """Traverses grid looking for 's' counters"""
        for row in range(len(player.ocean.ocean)):
            for col in range(len(player.ocean.ocean)):
                if player.ocean.ocean[row][col] == "S":
                    ship_counters += 1
        if ship_counters == 0:
            return True
        else:
            return False

    def clear_screen(self):
        input("\nNext Turn?")
        os.system('clear')

    def victory_message(self, winner, loser):
        print("\n\n\n*****************************************")
        print("%s's fleet has been destroyed, %s wins!" % (loser.name, winner.name))
        print("*****************************************")

battleshipscomp.py

from battleshipspvp import BattleshipsPVP
from computer import Computer
from player import Player


class BattleshipsCOMP(BattleshipsPVP):

    def __init__(self):
        start = input("Begin? (y or n) -----> ")
        if start in ["y", "Y"]:
        self.playCOMP()
        else:
            print("Aborted...")

    def playCOMP(self):
        pname = input("Player 1, state your name! -----> ")
        p = Player(pname)
        p.set_fleet()
        p.view_console()
        self.clear_screen()

        c = Computer()
        print("Computer setting its fleet...")
        c.set_compu_fleet()
        self.clear_screen()

        flag = True
        while flag is True:
            p.strike(c)
            if self.fleet_sunk(c) is True:
                self.victory_message(p, c)
                flag = False
            else:
                self.clear_screen()

                c.compu_strike(p)
                if self.fleet_sunk(p) is True:
                    self.victory_message(c, p)
                    flag = False
                else:
                    self.clear_screen()
        print("\nThanks for playing!")

playbattleships.py

from battleshipspvp import BattleshipsPVP
from battleshipscomputer import BattleshipsCOMP

# Script initiates game of battleships


def playbattleships():
    print("\n\n***********************")
    print("Welcome to Battleships!")
    print("***********************\n")

    print("\n 1) Player vs Player")
    print("\n 2) Player vs Computer")

    flag = True

    while flag:
        try:
            mode = int(input("\n\nPick a number to select a game mode ----> "))
            if mode == 1:
                flag = False
                BattleshipsPVP()
            elif mode == 2:
                flag = False
                BattleshipsCOMP()
            else:
                continue
        except ValueError:
            print("You can only pick either option 1 or 2")


playbattleships()

A view of the game board in play. The radar is the upper grid, the ocean is the lower grid.

. . . . . . . . . .
X . . . . . . . . .
. . . . . . . . . .
. . . . O . . . . .
. . . . . . . . . .
. . . . . . . . . .
. . . . . . . . . .
. . . . . . . . . .
. . . . . . . . . .
. . . . . . . . . .
|                 |
S ~ ~ ~ ~ ~ ~ ~ ~ ~
S ~ ~ ~ ~ ~ ~ ~ ~ ~
S ~ ~ ~ ~ ~ ~ ~ ~ ~
S ~ ~ ~ ~ ~ ~ ~ ~ ~
S ~ ~ ~ ~ S S S S ~
~ ~ ~ ~ ~ ~ ~ ~ ~ ~
~ ~ ~ ~ ~ S S X ~ ~
~ ~ S ~ ~ ~ ~ ~ ~ ~
~ ~ S ~ ~ ~ ~ ~ ~ ~
~ ~ ~ ~ ~ ~ ~ ~ ~ ~

Answer 1

Use your own Interfaces

You've created both your Ocean class and Radar class to accept a point key in the __getitem__ and __setitem__ methods. Ie)

def __getitem__(self, point):
    row, col = point
    return self.ocean[row][col]

def __setitem__(self, point, value):
    row, col = point
    self.ocean[row][col] = value

But in your Player class, you avoid using those interfaces, and instead directly reach into the Ocean and Radar classes and manipulate the internals directly:

        if self.ocean.valid_row(row) and self.ocean.valid_col(col):
            if target.ocean.ocean[row][col] == "S":
                print("DIRECT HIT!!!")
                target.ocean.ocean[row][col] = "X"
                target.register_hit(row, col)
                self.radar.radar[row][col] = "X"

You should be using the interfaces you created:

        if self.ocean.valid_row(row) and self.ocean.valid_col(col):
            if target.ocean[row, col] == "S":
                print("DIRECT HIT!!!")
                target.ocean[row, col] = "X"
                target.register_hit(row, col)
                self.radar[row, col] = "X"

Mark internals private

There is no "private" in Python, but by convention, if a member starts with a leading underscore, it should only be accessed by self. So using self._ocean is ok, but using self._ocean._ocean would not be, as the second ._ocean is reaching into another objects privates.

Ie:

class Ocean:
    def __init__(self, width=10, height=10):
        self._ocean = [["~" for i in range(width)] for i in range(height)]

So when you create Player, and you type self.ocean._ocean, you can think to yourself, "wait ... I shouldn't be using that ._ocean member of that self.ocean object; I should be calling a public method on self.ocean instead."

Don't use Recursion for an Input Loop

Title says it all:

def strike(self, target):
    try:
        ...
                    print("Area already hit....Check your radar!")
                    self.strike(target)
        ...
    except ValueError:
        print("You need to provide a number....\n")
        self.strike(target)
    ...

If you repeatedly enter an already hit location, your stack will eventually overflow. If you repeatedly enter an invalid row/col value, and get a ValueError exception, you'll get stack overflow while handling a ValueError, while handling a ValueError, while handling a ValueError, while handling a ValueError, while handling a ValueError, while ... a few thousand levels deep!

You did things better in set_fleet() with a while flag: loop.

os.system('clear')

Don't use this. Don't even use:

os.system('cls' if sys.platform == 'nt' else 'clear')

That is forking a new process, loading a shell processor, interpreting a command, which may launch yet-another-process, which clears the screen. A little heavy weight. And it is risky, if an executable, like cls.bat or clear is on the path; you could end up executing arbitrary instructions.

Use:

import colorama

def clear_screen():
    print(colorama.ansi.clear_screen())

colorama.init()

clear_screen()

Look at colorama for other interesting things you can do. Like making your ocean blue, your ships green, and your hits red.

Fixed Formatting

    print("|                 |")

What happens if your ocean is 8x8? Or 12x12? Will your vertical bars still properly line up?

    print("|" + " " * (2 * cols - 3) + "|")

would work better ... assuming cols is defined or replaced with something suitable.

Ditto with:

input("Pick a coordinate between 0 and 9 for the columns and rows on your board")

(Which as mentioned by Reinderien, should be a print statement.) What if you changed your board size?

Player-vs-Player or Player-vs-Computer

You've duplicated too much code. Player -vs- Player, Player -vs- Computer, and Computer -vs- Computer can all be handled by 1 "game" function.

def play_game(side1, side2):

    side1.set_fleet()
    side2.set_fleet()

    while True:
        side1.strike(side2)
        if self.fleet_sunk(side2):
            self.victory_message(side1, side2)
            break
        else:
            side1, side2 = side2, side1

You games would be launched like:

p1 = Player(p1name)
p2 = Player(p2name)
play_game(p1, p2)

or

p1 = Player(p1name)
p2 = Computer()
play_game(p1, p2)

For this to work, Computer would need a set_fleet() method, instead of a set_compu_fleet() method, as well as a strike() method instead of a compu_strike() method.

f-strings

If using Python 3.6+, instead of:

    print("%s's %s has been sunk!" % (self.name, boat.ship_type))

use:

    print(f"{self.name}'s {boat.ship_type} has been sunk!")

Antipatterns

Avoid is True. For example, while flag is True:. Use while flag:

Avoid if condition: return True else: return False. Use, return condition. For example:

    if ship_counters == 0:
        return True
    else:
        return False

should be:

    return ship_counters == 0

Avoid for idx in range(len(item)): blah(item[idx]). Use for val in item: blah(val).

For example:

ship_counters = 0
for row in range(len(player.ocean.ocean)):
    for col in range(len(player.ocean.ocean)):
        if player.ocean.ocean[row][col] == "S":
            ship_counters += 1

Would be better written as:

ship_counters = 0
for ocean_row in player.ocean.ocean:
    for cell in ocean_row:
        if cell == "S":
            ship_counters += 1
return ship_counters == 0

Or better:

ship_counters = sum(1 for row in player.ocean.ocean for cell in row if cell == "S")
return ship_counters == 0

Or even better:

return all(cell != 'S' for row in player.ocean.ocean for cell in row)

Docstrings

I disagree with @Reinderien's point:

This comment:
# Function uses player input to set up fleet positions on a player board..
# For each ship, a ship object containing relevant coordinates is appended to self.fleet.
belongs on the first line inside the function, in triple quotes.

The above is a comment, not a docstring.

• A # comment is used to document code for the reader of the code.

• A """docstring""" is used to provide usage documentation for a caller who may not have access to the source code.

  class Player:

  # Function uses player input to set up fleet positions on a player board.
  # For each ship, a ship object containing relevant coordinates is appended to self.fleet
  
  def set_fleet(self):
      """
      Ask the user to assign locations to their fleet's ships.
  
      Parameters: None
      Returns: Nothing
      """
  

Now, type help(Player.set_fleet). You will see the doc-string help, which is designed to help the caller. From this description, the caller would know that they don't need to write fleet = player.set_fleet(), since nothing is returned.

In contrast, someone reading the code (maybe to debug it, modify it, enhance it) will want to know what the code does. Appending a ship object to self.fleet is useful internal details the caller may not need to know.

See also sphinx-doc.org for an automated docstring processing utility which can generate HTML doc pages, pdf help documents, etc., all by reading the docstrings directly from the code.

Answer 2

Type hints

PEP484 type hints will help document and test your code. For example,

def can_use_col(self, row, col, size):

can be (I guess)

def can_use_col(self, row: int, col: int, size: int) -> bool:

In-place addition

col = col + 1

can be

col += 1

Logic-by-exception

This:

    try:
        self.ocean[row]
        return True
    except IndexError:
        return False

can simply be

return row in self.ocean

can_use_col

This method shouldn't need to form a list. Let's see if we can improve it.

First: replace i with _ since you don't use it. Also, instead of valid_coords as a list, you can maintain it as a count starting at 0. Increment it instead of appending to a list.

Also, don't repeat yourself - only do col += 1 once, in the scope above where you have it now.

Use a boolean directly by writing return size == len(valid_coords) instead of using an if statement. The same goes for this code:

    if self.coords == []:
        return True
    else:
        return False

Iteration

    for i in range(size):
        self.ocean[row][col] = "S"
        row = row + 1

Again, you don't use i, but you're also maintaining an index in row. Instead:

    for i in range(size):
        self.ocean[row + i][col] = "S"

This also solves the issue that you're changing an argument to your function, which you generally shouldn't.

Docstrings

This comment:

# Function uses player input to set up fleet positions on a player board.
# For each ship, a ship object containing relevant coordinates is appended to self.fleet

belongs on the first line inside the function, in triple quotes.

Typo

"Boats are placed form right to left."

form = from.

Input prompts

   input("Boats are placed form right to left.")

I'm not sure what this is asking the user to input, and they won't be either. Did you mean to write print here?

Character comparison

if orientation in ["v", "V"]:

If you actually needed to compare against multiple things, you should use a set instead of a list:

if orientation in {"v", "V"}:

However, your case is easier than that:

if orientation.lower() == 'v':

Answer 3

files

Python is not Java. Multiple classes can be in the same module (file)

grid

There is a lot of repetition between Ocean and Radar. You can use a lot of overlapping code

enum

instead of "." or "~" to denote what is in the grid, you could use Enums, and let the board representation method (view_radar for example) take care of how to represent this.

collection types

You only seem to use lists. Even when you only use the collection soo see whether something is in it, and order is not important. sets or dicts can be more suited on some moments. Get to know when to use which type of collection

orientation

You have a lot of methods that are the same, but one is for horizonatal, the other for vertical. This can be tackled by adding a parameter orientation (Which should be an Enum) to the method.

def plot_vertical(self, row, col):
    for i in range(self.size):
        self.coords.append((row, col))
        row = row + 1


def plot_horizontal(self, row, col):
    for i in range(self.size):
        self.coords.append((row, col))
        col = col + 1

can then be replaced by:

import enum

class Orientation(enum.Enum):
    Horizontal = (1, 0)
    Vertical = (0, 1)


def plot(self, row, col, orientation):
    d_col, d_row = orientation.value
    self.coords.extend(
        (row + i * d_row, col + i * d_col) for i in range(self.size)
    )

Even better would be to incorporate this in the ship's __init__, so the 2 steps in the placing of the fleet:

boat = Ship(ship, size)
boat.plot_vertical(row, col)

can become :

class Ship:

    def __init__(self, ship_type, size, position, orientation):
        self.ship_type = ship_type
        self.size = size
        row, col = position
        d_col, d_row = orientation.value
        self.coords = [(row + i * d_row, col + i * d_col) for i in range(self.size)]

and be called:

boat = Ship(ship, size, (row, col), orientation)