It's fabulous that you are using ast.literal_eval()
instead of eval()
. You shouldn't be naming it eval
, however, because you are shadowing the builbuilt-in function. If sometime later you decide that you want to use eval()
for some reason, you might be surprised if it doesn't work how you expect it to.
ConstantsYou use constants instead of magic values! board_size
and offsets
are both constants and should be ALLCAPS, but you aren't using magic values!
Yay! CodeThat's good. Your main code is in a function instead of at module level!.
Not so yay. WhatWhat if the user typed yes
? It would accept it, but treat it as no. You should validate the input before continuing. You might want to create a function that keeps asking until y
or n
is given. You might get more fancy and go with yes
, yeah
, uh huh
, yeah, man!
, ... whatever you want to do.
It's fabulous that you are using ast.literal_eval()
instead of eval()
. You shouldn't be naming it eval
, however, because you are shadowing the buil-in function. If sometime later you decide that you want to use eval()
for some reason, you might be surprised if it doesn't work how you expect it.
Constants instead of magic values! board_size
and offsets
are both constants and should be ALLCAPS, but you aren't using magic values!
Yay! Code is in a function instead of at module level!
Not so yay. What if the user typed yes
? It would accept it, but treat it as no. You should validate the input before continuing. You might want to create a function that keeps asking until y
or n
is given. You might get more fancy and go with yes
, yeah
, uh huh
, yeah, man!
, ... whatever you want to do.
It's fabulous that you are using ast.literal_eval()
instead of eval()
. You shouldn't be naming it eval
, however, because you are shadowing the built-in function. If sometime later you decide that you want to use eval()
for some reason, you might be surprised if it doesn't work how you expect it to.
You use constants instead of magic values! board_size
and offsets
are both constants and should be ALLCAPS, but you aren't using magic values!
That's good. Your main code is in a function instead of at module level.
What if the user typed yes
? It would accept it, but treat it as no. You should validate the input before continuing. You might want to create a function that keeps asking until y
or n
is given. You might get more fancy and go with yes
, yeah
, uh huh
, yeah, man!
, ... whatever you want to do.
from ast import literal_eval as eval
It's fabulous that you are using ast.literal_eval()
instead of eval()
. You shouldn't be naming it eval
, however, because you are shadowing the buil-in function. If sometime later you decide that you want to use eval()
for some reason, you might be surprised if it doesn't work how you expect it.
board_size = 8
BLACK = '\u26AB'
WHITE = '\u26AA'
EMPTY = '\u2B1c'
offsets = ((0,1),(1,1),(1,0),(1,-1),(0,-1),(-1,-1),(-1,0),(-1,1))
Constants instead of magic values! board_size
and offsets
are both constants and should be ALLCAPS, but you aren't using magic values!
def main():
Yay! Code is in a function instead of at module level!
if input('Display instructions? [Y/N]: ').lower() == 'y':
Not so yay. What if the user typed yes
? It would accept it, but treat it as no. You should validate the input before continuing. You might want to create a function that keeps asking until y
or n
is given. You might get more fancy and go with yes
, yeah
, uh huh
, yeah, man!
, ... whatever you want to do.
print('{token} is the winner!' % (BLACK if black>white else WHITE))
It looks like you're a little confused about what you're gonna do. That throws a TypeError
. Either use '{token} ...'.format(token=...)
or '%s...' % ...
, but don't try to merge them. I prefer .format()
, but you can do what you want.
return
It's useless. Take it out. You're already at the end of a function that doesn't return anything above, so it will return None
implicitly. There is no need to add the extra line.
...'''\
)
Unclosed parentheses are also line-continuers. You don't need the backslash. You can remove all of those backslashes in that function. By the way, I'm very pleased to see that you aren't using magic values. You use board_size
instead of writing in the actual numbers.
board = [[EMPTY for x in range(board_size)] for x in range(board_size)]
It's great that you are using list comprehensions. I would use _
instead of x
just to make it a little more obvious that it isn't being used.
def print_board(board):
for row in range(len(board)):
print(*board[row], sep='')
return
You aren't using row
for anything except as an index for board
. You should just iterate through board
in the first place:
for row in board:
print(*row, sep='')
My second problem is again the return
.
while(True):
The parentheses are useless. Just do while True:
You can change that in several places.
I would highly recommend classes as pointed out by @kushj. Even if you changed just that, your code would look very good.