4
\$\begingroup\$

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()
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Aug 12, 2018 at 10:38
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

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!

answered Aug 13, 2018 at 6:25
\$\endgroup\$
0
\$\begingroup\$

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())
)
answered Aug 13, 2018 at 19:41
\$\endgroup\$
2
  • 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 the board() function and not outside of it. \$\endgroup\$ Commented Aug 14, 2018 at 8:04
  • \$\begingroup\$ leaving it here for as an idea springer, but see @C. Harley's comment above \$\endgroup\$ Commented Aug 15, 2018 at 10:08

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.