patternpythonModerate
Python Connect-Four
Viewed 0 times
fourconnectpython
Problem
I am currently using Python 3.5.2 to create a connect four basic game. My game should work for two players and tell you when you have won, be it horizontal vertical or diagonal. However I do not know how to make my code smaller or more efficient, is there a way to cut down, or is my method just not very efficient in itself?
I have tried looking up various other ways of doing this code, alas I found them difficult to understand. My code does not have any errors I have picked up on, and though I have completed the task I set myself, I do not feel this is the best way to do it.
If anybody had any suggestions, that would be much appreciated. If I knew how to make it shorter without losing functionality I would do so.
I hope my code will help anybody else attempting the same task as mine is rather basic.
```
import time
grid1 = [0,0,0,0] # bottom row
grid2 = [0,0,0,0]
grid3 = [0,0,0,0]
grid4 = [0,0,0,0]
grids = [grid1,grid2,grid3,grid4]
check = []
user = 1
class fullSlot_error (Exception):
pass
def hasWon_def():
print ("player "+str(user)+" has won")
time.sleep (1)
def grid_def():
print ("",grid4,"\n",grid3,"\n",grid2,"\n",grid1)
def user_def():
global user
if user < 2:
user = 2
else:
user = 1
return user
def slotFull_def():
while True:
try:
if grid4[userInput -1] != 0:
raise fullSlot_error
else:
break
except fullSlot_error:
print ("slot is full try again")
confirm_def()
def confirm_def():
looop= True
while looop== True:
try:
global userInput
userInput = int(input("\ninput a slot player "+str(user)+"(1,4)\n"))
if userInput < 5 and 0 < userInput:
looop = False
else:
print ("invalid int")
except ValueError:
print ("invalid input")
def placement_def():
counter = 0
for i in range (0,4):
I have tried looking up various other ways of doing this code, alas I found them difficult to understand. My code does not have any errors I have picked up on, and though I have completed the task I set myself, I do not feel this is the best way to do it.
If anybody had any suggestions, that would be much appreciated. If I knew how to make it shorter without losing functionality I would do so.
I hope my code will help anybody else attempting the same task as mine is rather basic.
```
import time
grid1 = [0,0,0,0] # bottom row
grid2 = [0,0,0,0]
grid3 = [0,0,0,0]
grid4 = [0,0,0,0]
grids = [grid1,grid2,grid3,grid4]
check = []
user = 1
class fullSlot_error (Exception):
pass
def hasWon_def():
print ("player "+str(user)+" has won")
time.sleep (1)
def grid_def():
print ("",grid4,"\n",grid3,"\n",grid2,"\n",grid1)
def user_def():
global user
if user < 2:
user = 2
else:
user = 1
return user
def slotFull_def():
while True:
try:
if grid4[userInput -1] != 0:
raise fullSlot_error
else:
break
except fullSlot_error:
print ("slot is full try again")
confirm_def()
def confirm_def():
looop= True
while looop== True:
try:
global userInput
userInput = int(input("\ninput a slot player "+str(user)+"(1,4)\n"))
if userInput < 5 and 0 < userInput:
looop = False
else:
print ("invalid int")
except ValueError:
print ("invalid input")
def placement_def():
counter = 0
for i in range (0,4):
Solution
Some suggestions:
So here is my code:
Note that this could be simplified even further using numpy` arrays, but that is more advanced.
- It is generally recommended to follow the pep8 style for Python code. For example no space between function names and paranthesis,
lower_case_function_names, spaces after commas, etc.
- You can define your grid in a single operation rather than 5.
- For
print, you can use*to unpack your nested list, then usesep='\n'to put each row on a different line.
- It would be better to have at least one top-level function, rather than doing everything in the root of the script.
- You don't need global variables, it is better to pass the variables as arguments and return the result.
- For print, if you give multiple values as arguments, spaces are automatically inserted between the arguments.
- You don't get any benefit from using a custom exception here, just use a
breakin youriftest.
- you can use one-line
iftests (called "ternary expressions") to simplify some of your code.
- There isn't much benefit from a simple one-line function that is only used in one place. It makes things harder to read and slows the code down a tiny bit. Just inline the code.
- There isn't much point using a sentinel variable (like
looop) when you just assignFalseto it at a certain point. Just usebreak, andcontinueif need be to skip later parts of the loop.
- It is better to put the absolute minimal amount of code possible in your
tryblock to avoid accidentally catching the wrong exception.
- you can combine multiple comparisons, such as
0
- You can use return
instead ofbreakto exit out of awhileloop if you just want to return the result at that point with no further processing.
- You can use in
to test if a value is in a list or other sequence of values rather then usingand.
- You never use counter
inplacement_def.
- The 0
is implied inrange(0, x), so you can leave it out.
- Rather than using range
and indexing a list, it is easier to iterate over the list directly.
- You always subtract 1
fromuserInput. It would be easier to subtract one at the beginning.
- Rather than appending a list several times to another list, use extend
to append the all the elements at once.
- check
doesn't need to be global, its state is never shared between functions.
- You can use all
to simplify checking rows or columns, andzipto transpose lists.
- You can use any
andallto simplify your checks for whether a player has won and for whether the grid is full.
- You could simplify the code somewhat by having the check functions return a boolean, then printing in checks_def
.
- I don't think placement_def
is correct. You get a user input every time through the loop, rather than just once, and you overwrite the value if it is the current use or empty, when you want to overwrite only if empty. Further, you loop from top to bottom, when you want to loop from bottom to top.
- You hard-code the grid size, but it would be easy to allow the user to specify a square grid size.
So here is my code:
def play(n=None):
if n is None:
while True:
try:
n = int(input('Input the grid size: '))
except ValueError:
print('Invalid input')
continue
if n user_input or user_input > n+1:
print('invalid input:', user_input)
elif grids[0][user_input-1] != 0:
print('slot', user_input, 'is full try again')
else:
return user_input-1
def place_piece(user_input, user, grids):
for grid in grids[::-1]:
if not grid[user_input]:
grid[user_input] = user
return
def check_won(grids, user, n):
return any(all(cell == user for cell in grid) for grid in grids)
def diagcheck_won(grids, user, n):
return all(grids[x][x] == user for x in range(n))
if __name__ == '__main__':
play()Note that this could be simplified even further using numpy` arrays, but that is more advanced.
Code Snippets
def play(n=None):
if n is None:
while True:
try:
n = int(input('Input the grid size: '))
except ValueError:
print('Invalid input')
continue
if n <= 0:
print('Invalid input')
continue
break
grids = [[0]*n for _ in range(n)]
user = 1
print('Current board:')
print(*grids, sep='\n')
while True:
user_input = get_input(user, grids, n)
place_piece(user_input, user, grids)
print('Current board:')
print(*grids, sep='\n')
if (check_won(grids, user, n) or
check_won(zip(*grids), user, n) or
diagcheck_won(grids, user, n) or
diagcheck_won(grids[::-1], user, n)):
print('Player', user, 'has won')
return
if not any(0 in grid for grid in grids):
return
user = 2 if user == 1 else 1
def get_input(user, grids, n):
instr = 'Input a slot player {0} from 1 to {1}: '.format(user, n)
while True:
try:
user_input = int(input(instr))
except ValueError:
print('invalid input:', user_input)
continue
if 0 > user_input or user_input > n+1:
print('invalid input:', user_input)
elif grids[0][user_input-1] != 0:
print('slot', user_input, 'is full try again')
else:
return user_input-1
def place_piece(user_input, user, grids):
for grid in grids[::-1]:
if not grid[user_input]:
grid[user_input] = user
return
def check_won(grids, user, n):
return any(all(cell == user for cell in grid) for grid in grids)
def diagcheck_won(grids, user, n):
return all(grids[x][x] == user for x in range(n))
if __name__ == '__main__':
play()Context
StackExchange Code Review Q#134636, answer score: 11
Revisions (0)
No revisions yet.