Battleships OOP python

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

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


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

def __setitem__(self, point, value):
row, col = point

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 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.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:
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")

def view_console(self):
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)

else:
self.strike(target)
else:
print("Negative...")

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)

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)

else:
print("Missed....recalibrating")

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 ~ ~ ~ ~ ~ ~ ~
~ ~ ~ ~ ~ ~ ~ ~ ~ ~

• If you want to improve your strike function, I'd recommend this nice article – Grajdeanu Alex. Sep 25 '19 at 5:51

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)


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)


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:
...
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.

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:


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':

• Thanks man I really appreciate it. – user207830 Sep 24 '19 at 18:55
• return row in self.ocean will not work if the ocean is a list of lists. This can work if the ocean is a dict – Maarten Fabré Sep 25 '19 at 7:53

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)