The program had several unused items. I ignored and ultimately deleted them.
Stop using global variables and ignore OOP for now. Absolutely, stop using
global variables from this point forward. They are never needed except under
quite unusual circumstances -- none that you are likely to encounter at this
stage of your journey. And before dealing with OOP, first develop some
proficiency in building programs from small, focused, well-designed functions.
Until you do that, OOP will be at best a distraction and at worst a crutch
allowing you to smuggle global-variable thinking into your code while
pretending that you're not.
Functions: a few best practices. Build functions that take data as input
and return data as output. Whenever feasible, do not ask functions to mutate
the data they are given. The basic model: information in, information out, and
never mess with someone else's information.
Step 1: move the globals into print_board(). Try to run the program.
You'll encounter a series of problems. For example, first we see that
generate_ships() will have no access to board. The solution is to have the
function return the board that it creates. While we're doing that, we also move
the ship-counting logic to that function, because that's where the information
needs to be managed. Run the program again: you'll see that we need to pass
board down the call chain, but that's easy enough to do. After a few edits
like that, you'll have a running program.
Step 2: tidy up. Define constants for empty and full. Put spaces after commas
and other operators. For consistency, one should also define constants for the
directions, but that topic raises other issues, so let's defer that. And
rearrange the functions so that they agree with the narrative flow and
structural hierarchy of the program. Here's the code after those adjustments.
import random
EMPTY = '.'
FULL = 'x'
def print_board():
board = generate_ships(4)
for row in board:
print(' '.join(row))
def generate_ships(total_ships):
board = [[EMPTY]*10 for i in range(10)]
ships_placed = 0
rows, cols = len(board), len(board)
while ships_placed != total_ships:
random_row = random.randint(0, rows - 1)
random_col = random.randint(0, cols - 1)
direction = random.choice(["left", "right", "up", "down"])
ship_size = random.randint(3, 5)
ships_placed += place_ships_helper(board, random_row, random_col, ship_size, direction)
return board
def place_ships_helper(board, row, col, length, direction):
start_row, end_row, start_col, end_col = row, row + 1, col, col + 1
if direction == "left":
if col - length < 0:
return False
start_col = col - length + 1
elif direction == "right":
if col + length >= len(board):
return False
end_col = col + length
elif direction == "up":
if row - length < 0:
return False
start_row = row - length + 1
elif direction == "down":
if row + length >= len(board):
return False
end_row = row + length
return place_ships(board, start_row, end_row, start_col, end_col)
def place_ships(board, start_row, end_row, start_col, end_col):
for r in range(start_row, end_row):
for c in range(start_col, end_col):
if board[r][c] != EMPTY:
return False
for r in range(start_row, end_row):
for c in range(start_col, end_col):
board[r][c] = FULL
return True
if __name__ == "__main__":
print_board()
Step 3. assess. Things are considerably better: we have no global
variables! But our functions are defined entirely around mutation, which we'd
like to avoid. And our modeling of directions data seems awkward, resulting in
four chunks of semi-repetitive code.
Step 4. directions as change values. The noted problems spring
from a poor modeling of data. Currently, we are using English words to
represent directions, but those words have no connection to the underlying data
used by the program. We have a board with rows, columns, and cells. Using the
language of such data, how does one represent a direction? There are various
options, but one common approach is to represent a direction as pairs of
(row_change, col_change).
Step 5. stop mutating. Rather than asking lower-level functions to mutate
the board that we give them, we should build the board up at the top level by
asking lower-level functions go give us helpful data to achieve that. One
approach is to ask a lower-level function to generate a single valid ship. How
do we represent a ship in terms of the data? Again, a sequence of pairs will
work, specifically (row, col) pairs. So print_board() will ask
generate_board() to return the board and ships that it creates. To do that,
generate_board() will ask generate_ship() for a ship until there are enough
of them. And generate_ship() will delegate secondary calculations to utility
functions, in particular the check for whether a needed cell is both valid and
empty.
import random
EMPTY = '.'
FULL = 'x'
DIRECTIONS = (
(0, 1), # Right
(0, -1), # Left
(1, 0), # Up
(-1, 0), # Down
)
MIN_SIZE = 3
MAX_SIZE = 5
def print_board():
board, ships = generate_board(10, 4)
for row in board:
print(' '.join(row))
def generate_board(board_size, n_ships):
# A board without ship information is ambiguous so return both.
board = [[EMPTY] * board_size for _ in range(board_size)]
ships = []
while len(ships) < n_ships:
row = random.randint(0, board_size - 1)
col = random.randint(0, board_size - 1)
direction = random.choice(DIRECTIONS)
ship_size = random.randint(MIN_SIZE, MAX_SIZE)
ship = generate_ship(board, row, col, ship_size, direction)
if ship:
ships.append(ship)
for r, c in ship:
board[r][c] = FULL
return (board, ships)
def generate_ship(board, row, col, ship_size, direction):
r = row
c = col
dr, dc = direction
cells = []
for _ in range(ship_size):
if is_empty(board, r, c):
cells.append((r, c))
r += dr
c += dc
else:
return None
return cells
def is_empty(board, row, col):
try:
return min(row, col) >= 0 and board[row][col] == EMPTY
except IndexError:
pass
return False
if __name__ == "__main__":
print_board()
