I made a battleship game for a programming class, where I only had about a week to get it done. Under this time crunch, and the fact that I don't know python very well, I made some of the code in very roundabout ways, and probably far more complicated than it needs to be.
"""
This is my program that lets you play one man battleship.
It has a board that displays the info to the player, henceforth know as the player board,
and an enemy board which is hidden from the player, know as the enemy board which is used to store ship locations.
There is also an info board that has the ship locations for each board on the corosponding line.
On the enemy board, a zero is used to represent an empty tile, while an X is a ship section.
On the player board, a zero is used to represent a unguessed tile, an M the misses, and an X for hits.
"""
import random
board = []
hits = 0
random.seed()
num = random.randrange(1, 5)
sunk = [0 , 0, 0, 0]
# Loads the enemy board
def enemyBoardLoader():
global num
file = open("board" + str(num) + ".txt")
return file.read()
#Loads the line of the info board that corosponds to the board chosen
def boardInfoLoader():
file = open("boardInfo.txt")
#Reads a line and puts it on a filler varible, as that is the only way to incremeant the readline command
for x in range(num - 1):
filler = file.readline()
return boardInfoInterp(file.readline())
#Turn the line input into a list of lists
def boardInfoInterp(boardInfo):
boardInfoList = [] #Output
shipSet = [] #Varible 1 that is used to chunk the list
shotSet = [] #Varible 2 that is used to chunk the list
x = 0
#Loop that moves it into a nested list
while x < len(boardInfo):
#If the letter the program is on is a number, it adds it to the second chunk varible
try:
shotSet.append(int(boardInfo[x]))
#If its not, it runs this code
except ValueError:
#If it reaches a comma, moves the first chunk varible to the first one, then clears the first chunk varible
if boardInfo[x] == ",":
shipSet.append(shotSet)
shotSet = []
#If it reaches a semi colon, moves the second chunk varible to the ouput, making a nested list, then clears the second chunk varible
elif boardInfo[x] == ";":
boardInfoList.append(shipSet)
shipSet = []
x += 1
return(boardInfoList)
#If a shot is on a ship, it removes that shot from the boardInfo list
def boardInfoRemover(hit):
global boardInfo
#Makes the input be in a format that the program needs
hit = str(hit)
hit = list(hit)
#Removes extra charecters from the list
while "[" in hit:
hit.remove("[")
while " " in hit:
hit.remove(" ")
while "," in hit:
hit.remove(",")
while "]" in hit:
hit.remove("]")
x = 0
#Makes the numbers left in the list numbers
while x < len(hit):
hit[x] = int(hit[x])
x += 1
x = 0
#Checks to see which shot location it was, and removes it from the boardInfo list
while x < len(boardInfo):
y = 0
while y < len(boardInfo[x]):
if boardInfo[x][y] == hit:
del boardInfo[x][y]
y += 1
x += 1
# General things done to the player board
def boardFunc(func, shot):
global board
if func == "reset": # Checks if command given was reset
# Sets default board state of 100 zeros
for x in enemyBoard:
board.append("0")
if func == "print": # Checks if command given was print
# Prints 10 rows of 10 characters
for x in range(10):
print(" ".join(board[(x * 10):((x + 1) * 10)]))
if func == "update": # Checks if command given was update
if enemyBoard[((shot[0] - 1) + (shot[1] - 1) * 10)] == "X": # If the shot location is an X on the enemy board, mark a hit on the player board
if board[((shot[0] - 1) + (shot[1] - 1) * 10)] != "X":
board[((shot[0] - 1) + (shot[1] - 1) * 10)] = "X"
boardInfoRemover(shot)
global hits
hits += 1
else:
board[((shot[0] - 1) + (shot[1] - 1) * 10)] = "M" # If the shot location is not an X on the enemy board, mark a miss on the player board
#Takes and cleans the input to prevent errors
def inputCleaner():
global y, x
num = []
repeat = True # Secondary variable to handle loops
# Checks to make sure input is 2 items instead of one
while repeat:
try:
xy = input("Were do you want to shoot your shot? (num1 num2):")
x, y = xy.split() # Gives error if its only one number
repeat = False # If there is an error, this does not run, making the loop keep running
# If error is given, runs this instead of any code left to run
except ValueError:
print("Please enter two numbers")
true = True # Primary variable to handle loops
# Main loop to check if input is right
while true:
true = False # *Starts chuckling*
try:
# Sets the variable to numbers instead of characters they came in as. If this fails, it runs the except code below
x = int(x)
y = int(y)
if y > 10 or y < 1: # Makes sure that the first number is on the board, if not asks for another number to replace it
true = True
y = input(
"Please make sure that your first number is at least 1 and no more than 10. Re-enter only your first number:")
elif x > 10 or x < 1: # Same as above, but with x
true = True
x = input(
"Please make sure that your second number is at least 1 and no more than 10. Re-enter only your second number:")
else: # If neither of the above are true, puts the numbers in the num list
num.append(int(x))
num.append(int(y))
except ValueError or TypeError: # If something other than a number is entered, it runs this code
repeat = True
# This code makes sure at least two numbers were entered
while repeat:
try:
xy = input("Please use only numbers. Enter you shot location again:")
x, y = xy.split()
repeat = False
true = True
except ValueError:
print("Please enter two numbers")
return num
#Checks if all locations on a ship have been shot
def sunkChecker():
global boardInfo
global sunk
if len(boardInfo[0]) == 0 and sunk[0] == 0:
print("You sunk the aircraft carrier.")
sunk[0] = 1
if len(boardInfo[1]) == 0 and sunk[1] == 0:
print("You sunk the battleship.")
sunk[1] = 1
if len(boardInfo[2]) == 0 and sunk [2] == 0:
print("You sunk the cruiser.")
sunk[2] = 1
if len(boardInfo[3]) == 0 and sunk[3] == 0:
print("You sunk the submarine.")
sunk[3] = 1
enemyBoard = enemyBoardLoader()
boardInfo = boardInfoLoader()
while True:
boardFunc("reset", 0)
while hits < 15: # Each board has 15 ship spots, when all are hit it breaks out of this loop
boardFunc("print", 0)
boardFunc("update", inputCleaner())
sunkChecker()
boardFunc("print", 0)
if input("YOU WON!! Would you like to play again? (y/n):") == "n":
quit()
What are some parts that could be simplified? Also, these are the specific files I have for this program.
-
7\$\begingroup\$ Please see What to do when someone answers. I have rolled back Rev 3 → 2 \$\endgroup\$Sᴀᴍ Onᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2019年11月12日 22:43:41 +00:00Commented Nov 12, 2019 at 22:43
3 Answers 3
I think the place to start is the function boardFunc
. Until you get comfortable with "functional programming" and dependency inversion, you should probably never write a function that takes an argument telling it what to do. We can rewrite that function as
# Set up the player board for a new game. (100 "0"s)
def boardReset():
global board
for x in enemyBoard:
board.append("0")
# Print the player board. (10 rows of 10)
def boardPrint():
global board
for x in range(10):
print(" ".join(board[(x * 10):((x + 1) * 10)]))
# Update the player board based on a shot
def boardUpdate(shot):
global board
if enemyBoard[((shot[0] - 1) + (shot[1] - 1) * 10)] == "X": # If the shot location is an X on the enemy board, mark a hit on the player board
if board[((shot[0] - 1) + (shot[1] - 1) * 10)] != "X":
board[((shot[0] - 1) + (shot[1] - 1) * 10)] = "X"
boardInfoRemover(shot)
global hits
hits += 1
else:
board[((shot[0] - 1) + (shot[1] - 1) * 10)] = "M" # If the shot location is not an X on the enemy board, mark a miss on the player board
I'm testing your code in Jupyter Notebooks, where the while True: ... quit()
pattern doesn't work right. We can make it a little more explicit, reliable, and portable by using a mutable variable to break the loop:
keepPlaying = True
while keepPlaying:
boardReset()
while hits < 15: # Each board has 15 ship spots, when all are hit it breaks out of this loop
boardPrint()
boardUpdate(inputCleaner())
sunkChecker()
boardPrint("print", 0)
keepPlaying = "y" == input("YOU WON!! Would you like to play again? (y/n):")
In fact, you seem a bit trigger-happy with the while
loops in general. If you can use a for
loop, that's preferable. When you want to loop over a range of numbers, you can use range
, but when you're working with an existing list you're better off looping over the list itself (or a derivation of the list). Even better is to avoid a loop altogether. While you're at it, try to work with the data structures python provides!
(I was able to confirm that your boardInfoRemover
function was working for hits in the tenth row or column, but I can't explain how.)
#If a shot is on a ship, it removes that shot from the boardInfo list
def boardInfoRemover(hit):
#pprint.pprint(hit)
global boardInfo
#Checks to see which shot location it was, and removes it from the boardInfo list
for i, boardRow in enumerate(boardInfo):
for j, value in enumerate(boardRow):
if value == hit:
del boardInfo[i][j]
Looking at boardReset
, does it actually reset the board, or does it just assume it's already empty, and then append? I was just saying that you should use fewer loops, so now's a good time to introduce list comprehensions. (The other option would be to map with a lambda, build a const function, but comprehension are a good tool.)
# Set up the player board for a new game. (100 "0"s)
def boardReset():
global board
board = ["0" for _ in enemyBoard]
The inputCleaner
function is large, but without bringing in a new library to do input cleaning for you, there may not be a way to make it shorter. What we can do is make it's flow more clear (linear), not rely on exceptions, and return 0-indexed coordinates. In particular, we can make it recursive, avoiding a lot of the looping behavior. We loose the part where the user only has to re-enter one of the numbers if they're out of range, but that doesn't seem like a big loss.
#Takes and cleans the input to prevent errors
def inputCleaner():
failures = []
coordinates = None # be careful about None, but it'll do for now.
input_list = input("Were do you want to shoot your shot? (num1 num2):")
if 2 != len(input_list):
failures.append("Please enter exactly two numbers.")
if any(not coordinate.isdigit() for coordinate in list_input):
failures.append("Please use only numbers.")
else:
coordinates = map(int, input_list)
local_names = ["first", "second"]
for i, c in enumerate(coordinates):
if 10 < c or 1 > c:
failures.append("Please make sure that your {} number is at least 1 and no more than 10."
.format(local_names[i]))
if not failures:
return [c - 1 for c in coordinates]
# If we get here, failures will not be empty.
failures.append("Enter you shot location again.")
for f in failures:
print(failure)
return inputCleaner()
Again, use the tools the language gives you to simplify. This time the boardInfoInterp
function, because we need to convert it to match the 0-indexing we want to use everywhere. We want to use 0-indexing so that we can do list-lookup.
#Turn the line input into a list of lists of pairs (which are lists) of 0-indexed ints
def boardInfoInterp(boardInfo):
return [
[
[int(coordinate) - 1 for coordinate in pair.split()]
for pair
in ship.split(",")
]
for ship
in boardInfo.split(";")
]
Other things we should be doing:
- Use bools
- Store the board as a list of lists.
- Use arguments and return values instead of global variables.
But getting this to the point where there's no global state would amount to a complete rewrite. If you want to go that route, you'll need to think about how to best represent the game state as an abstract data structure. You'd then write functions like
newGame: ()->GameState
playerMove: (GameState, PlayerMove)->GameState
display: GameState->print
-
\$\begingroup\$ Thanks for the in-depth answer. The way the
boardInfoRemover
works is by taking the shot input (say,[10, 10]
) turns it into astr
("[10, 10]"
), turns thatstr
into alist
([ "[", "1", "0", ",", " ", "1", "0", "]"]
), removes the brackets spaces and commas (["1", "0", "1", "0"]
), then finally makes all the numbers intoint
([1, 0, 1, 0]
). Quite ridiculous but the first thing I thought of. \$\endgroup\$user211881– user2118812019年11月12日 22:16:58 +00:00Commented Nov 12, 2019 at 22:16 -
\$\begingroup\$ "you should probably never write a function that takes an argument telling it what to do" Why not? It can be very useful at times, even if you ignore the existance of functional programming the rest of the time. \$\endgroup\$2019年11月13日 14:05:17 +00:00Commented Nov 13, 2019 at 14:05
-
\$\begingroup\$ @Mast, There's probably a lot of useful discussion about it; higher-level functions are great, and dependency injection/inversion is its own whole thing that can be implemented a lot of different ways. That said, the original
boardFunc
is an excellent example of both when not to and how not to write a function that takes an argument telling it what to do: There's no overlap between the execution paths, the argument in question is a "data" Type when it's never used as "data", and the arg value is hard-coded in all of the places where it's used. \$\endgroup\$ShapeOfMatter– ShapeOfMatter2019年11月13日 22:06:43 +00:00Commented Nov 13, 2019 at 22:06
Just a quick comment for now. Unfortunately, except ValueError or TypeError
does not do what you think it does. Python tries to be readable English where possible, but here it doesn't quite work. The parser parses this statement as:
except (ValueError or TypeError)
where the second part follows the rules of the or
operator. If the first argument is truthy, return that truthy value, if not return the truthiness of the second argument.
For integers, which are falsey for 0
and truthy otherwise, this means:
1 or 2 -> 1
0 or 2 -> 2
0 or 0 -> 0
In addition, by default all objects are truthy if they exist (unless overwritten by the class itself). Since ValueError
exists, it is parsed like this:
except (ValueError or TypeError) -> except ValueError
In other words, this cannot actually catch a TypeError
:
try:
raise TypeError
except ValueError or TypeError:
print("caught!")
# TypeError: ...
Instead, use a tuple for multiple exceptions to be caught by the same except
statement:
try:
raise TypeError
except (ValueError, TypeError):
print("caught!")
# caught!
Be careful not to use the tuple without parenthesis, which meant something else in Python 2 and is a SyntaxError
in Python 3:
try:
raise TypeError
except ValueError, TypeError:
print("caught!")
# Python 2: TypeError: ...
# Python 3: SyntaxError: ...
was the same as
try:
raise TypeError
except ValueError as TypeError:
print("caught!")
# TypeError: ...
In other words, it overwrites the variable TypeError
with the specific caught ValueError
, which is not the exception being raised here.
Style
Code style is not "mission critical" for a beginner, but can become more important when working on larger projects. Python has an "official" Style Guide for Python Code, which is widely followed in the larger Python community. It codifies a lot of recommendations on variable naming, whitespace, and the like.
As an example, the official recommendation for variable and function names is to use lower_case_with_underscores
instead of camelCase
.
Documentation
The style guide also has a few rules about how function documentation should be written in order to be usable by Python's built-in help(...)
. The convention is to have the docstring after the def
-line, """quoted in triple quotes"""
.
def enemyBoardLoader():
"""Loads the enemy board"""
global num
file = open("board" + str(num) + ".txt")
return file.read()
Random number generation
You only really need to call random.seed(...)
if you actually want to get reproducible pseudo-random numbers. Then you would pass a fixed seed value. If you only want some pseudo-random numbers, you don't need to call it, because the random number generator is seeded by default.
Also, there seems to be a small bug in your code. Since what really want are numbers between 1 and 5 (the boards in your repo are numbered 1 to 5), you will have to use random.randrange(1, 6)
or even better random.randint(1, 5)
, since otherwise board 5 will never be selected.
Global variables
You are using global variables in a lot of places, which makes it harder to see which functions modify what part of the game state, and as a consequence of that also harder to test functions individually without resetting the global state in the mean time.
Fortunately, quite some instances of global variable usage in your code can be replaced by using (additional) function arguments. E.g. instead of
def enemyBoardLoader(): global num file = open("board" + str(num) + ".txt") return file.read()
simply do
def enemyBoardLoader(num):
file = open("board" + str(num) + ".txt")
return file.read()
As you can see we where able to get rid of that pesky global
keyword. Sidenote: strictly speaking global
is not even necessary to read from variables from a surrounding scope, but only when trying to modify them. For instance, this can be witnessed in boardInfoLoader()
, where you are using num
without explicitly declaring it global
. But as I said, it's best to avoid it whenever possible.
The truth
Python has a bool
datatype with True
and False
as possible values, but that should be old news for you since you already use it in some places. Prefer to use True
instead of 1
and False
instead of 0
in sunk
and similar situations. Then instead of sunk[0] == 0
, you'd write sunk[0] == False
or even more "pythonic" not sunk[0]
(see how this starts to sound like plain English?).
Closing files
Whenever you open(...)
a file, don't forget to .close()
it properly. Otherwise this could get you into trouble in the long run. Since forgetting is so easy, Python has the so called with
-statement which automagically ensures that the file gets closed no matter what happens (i.e. not even an Exception
or return
can stop it from doing so). To get back to the previous example, now rewritten using a with
:
def enemyBoardLoader(num):
with open("board" + str(num) + ".txt") as file:
return file.read()
Reading the boards from disk
Reading and writing data from and to disk (aka (de)serializing) can be a tedious task as you maybe witnessed first-hand when writing boardInfoInterp()
and associates. Fortunately, Python can greatly simplify your life here. Maybe you have heard of the so called JSON format, very often found in web applications. Python has a module called json
which allows you to (de)serialize your data from/into a standardized format. As an example, say you have a list of lists (e.g. like your board), my_data = [list(range(4)) for _ in range(3)]
. Enter json
. json.load/dump
allows you two load or write data from and to files with relative ease:
with open("mydata.json", "w") as output_file:
json.dump(mydata, output_file)
mydata.json
now has the following content:
[[0, 1, 2, 3], [0, 1, 2, 3], [0, 1, 2, 3]]
Simply calling
with open("mydata.json", "r") as file_:
mydata = json.load(file_)
brings your data back to Python.
Using json
might be overkill for simple things like lists of lists, but it can save you a lot of headache once it gets a little more involved.
That's it for now. Maybe I have time to look at your code later again. Till then: Happy coding!