I wrote this program in Python 3.I would like to know if there is some more functionality that could be added/removed/is missing.Any comments would be helpful!
#Steps from user :
"""
Enter Input
P1 and P2
Each time for input will be preceded by a printed board.
The game will continue until an input matches a win combination.
"""
win_combinations = [
[1,2,3],
[4,5,6],
[7,8,9],
[1,4,7],
[2,5,8],
[3,6,9],
[1,5,9],
[3,5,7]]
tot_list = [1,2,3,4,5,6,7,8,9]
def board():
print ("{}|{}|{}".format(tot_list[0],tot_list[1],tot_list[2]))
print ("{}|{}|{}".format(tot_list[3],tot_list[4],tot_list[5]))
print ("{}|{}|{}".format(tot_list[6],tot_list[7],tot_list[8]))
p1 = 0
p2 = 0
def player_input():
global p1
global p2
while True:
try:
print("\n\n")
board()
print("\n")
p1 = int(input("P1(X): Enter your Input:"))
print("\n")
p2 = int(input("P2(O): Enter your Input:"))
if 0 < p1 < 10 and 0 < p2 < 10 :
assign_values(p1,p2)
break
print("Re-enter valid input")
except:
print("Invalid input")
continue
def assign_values(player1,player2):
if type(tot_list[player1-1]) == str() or type(tot_list[player2-1]) == str():
print("Re-enter valid input")
player_input()
if player1 in tot_list:
tot_list[player1-1] = "X"
if player2 in tot_list:
tot_list[player2-1] = "O"
p1_win = False
p2_win = False
tie = False
def default_values():
global tot_list
global p1
global p2
global p1_win
global p2_win
global tie
tot_list = [1,2,3,4,5,6,7,8,9]
p1 = 0
p2 = 0
p1_win = False
p2_win = False
tie = False
def play():
global p1_win
global p2_win
global tie
global tot_list
while True:
player_input()
for i in win_combinations:
if tot_list[i[0]-1] == "X" and tot_list[i[1]-1] == "X" and tot_list[i[2]-1] == "X":
p1_win = True
break
elif tot_list[i[0]-1] == "O" and tot_list[i[1]-1] == "O" and tot_list[i[2]-1] == "O":
p2_win = True
break
if all(isinstance(i,str) for i in tot_list) == True:
tie == False
if p1_win==True:
print("P1 has won!")
break
elif p2_win==True:
print("P2 has won!")
break
elif tie == True:
print("Its a tie!")
break
play_again = input("Play Again?(y/n)")
if play_again == "y":
default_values()
play()
else:
print("Ciao")
if __name__ == '__main__':
play()
2 Answers 2
Your code doesn't work like in real life. For example, when I play:
1|2|3
4|5|6
7|8|9
P1(X): Enter your Input:1
P2(O): Enter your Input:1
X|2|3
4|5|6
7|8|9
There is no error generated. Both players cannot request the same location. Also, playing a new game, this scenario:
P2(O): Enter your Input:5
X|O|3
X|O|6
7|8|9
P1(X): Enter your Input:7
P2(O): Enter your Input:
This scenario P2(O):
should never have been reached. Player 2 doesn't get another move after Player 1 wins the game. Recommend fixing these problems.
Onto your code -
Firstly, you have an entry point - that's good, however it doesn't do anything other than call play()
.
You should move the control loop into the entry point, and remove duplicate code scattered amongst your code. In default_values()
, you scatter those pieces across the code.
tot_list = [1, 2, 3, 4, 5, 6, 7, 8, 9]
p1 = 0
p2 = 0
p1_win = False
p2_win = False
tie = False
Yet you have them inside default_values()
. Inside your play()
function, you have play()
in the loop, but the first time you call it, you specify default_values()
- notice the inconsistency there?
You should have realized that when you were reviewing your code.
So, we fix that, and this is what we come up with:
if __name__ == '__main__':
win_combinations = [
[1, 2, 3],
[4, 5, 6],
[7, 8, 9],
[1, 4, 7],
[2, 5, 8],
[3, 6, 9],
[1, 5, 9],
[3, 5, 7]]
while True:
default_values()
play()
choice = input("Play Again? ('n' to exit):")
if choice == "n":
break
print("Ciao")
You also have lots of other issues with your code, but let me give you a few pointers for the rest (I don't want to rewrite your entire code).
if p1_win == True:
You don't need to specify "True" as this is the same: if p1_win:
You have global statements all throughout your code. This is bad - and the reason is - you don't know 100% where your variables are being changed. The #1 source of all bugs when coding is when the state changes in variables and the change was unintended. Does that make sense? Please review it by googling.
What you should do is pass variables around. Having a piece of code like:
active_player = not active_player
player_input = get_player_input()
board_state = update_board(player_input, board_state)
if check_for_a_winner(board_state):
print("We have a winner! {} wins!".format(active_player))
shows you how we inject the input as well as the current board status into the update_board
function, and receive the updated board status back from it. We can then present the current board status into the winner check function.
This examples shows that state changes only in very specific places, and if you're hunting down a bug, you can find it quite simply.
Typically, with several pieces of related state, you'll need an object which can represent all of them at once. I recommend reading a book which explains about classes, for instance having multiple dog and cat classes and how you can represent their attributes.
Please attempt to incorporate these changes into you code and repost your updated solution (with a link to this page) and demonstrate your code working better, with those bugs removed.
Good Luck!
just appending to @C. Harley. as you are aware of .format, you might want to use it in prints as well
from
print("\n\n")
board()
print("\n")
to
print(""""
{}
""".format(board())
)
-
1\$\begingroup\$ This print statement would ruin your indentation levels, but a fair call on injecting the board print statement.
print("\n\n{}\n".format(board())
would be preferred to your snippet, however if we're being proper, the new-lines are specific to printing the board, they belong inside theboard()
function and not outside of it. \$\endgroup\$C. Harley– C. Harley2018年08月14日 08:04:16 +00:00Commented Aug 14, 2018 at 8:04 -
\$\begingroup\$ leaving it here for as an idea springer, but see @C. Harley's comment above \$\endgroup\$Abdur-Rahmaan Janhangeer– Abdur-Rahmaan Janhangeer2018年08月15日 10:08:08 +00:00Commented Aug 15, 2018 at 10:08