patternpythonMinor
War Card Game Simulator
Viewed 0 times
simulatorgamewarcard
Problem
This is the second thing I've ever written in Python. I am just looking for what I could improve upon along with things that could be done better. It is as compact as I could make it with my current experience.
import random
def war_handler():
war_handler.count += 1
print "War is {0} is happening.".format(war_handler.count)
if len(player_one) > 4 and len(player_two) > 4:
limiter = 3
elif len(player_one) == 4 or len(player_two) == 4:
limiter = 2
else: # len(player_one) len(player_two):
limiter = len(player_two) - 2
else: # len(player_one) player_two[0]:
temp.append(player_one[0])
temp.append(player_two[0])
player_one.remove(player_one[0])
player_two.remove(player_two[0])
for y in range(0, len(temp)):
player_one.append(temp[y])
elif player_one[0] = 4 and len(player_two) >= 4:
war_handler.count = 0
war_handler()
total_wars += war_handler.count
# War sim over
# Simple determination of winner
elif player_one[0] != player_two[0]:
temp.append(player_one[0])
temp.append(player_two[0])
random.shuffle(temp)
player_one.remove(player_one[0])
player_two.remove(player_two[0])
if player_one > player_two:
player_one.append(temp[0])
player_one.append(temp[1])
else: # player_one < player_two
player_two.append(temp[0])
player_two.append(temp[1])
else:
continue
if not player_one:
print "Player {0} Wins!".format(player_two_name)
elif not player_two:
print "Player {0} Wins!".format(player_one_name)
print "Total Wars Completed: {0}".format(total_wars)Solution
Overall structure
The most important thing to improve is to decompose this code to multiple smaller functions. At the same time, remove all code in the global namespace, so that the program has this structure:
See the next section for a closely related tip.
Don't repeat yourself
This snippet is repeated twice to input cards:
The most common technique to reduce repeated logic is to use functions,
for example:
Then you could call this with for each player:
Improving the input of cards
The current method of inputting cards could use some improvement:
Applying the above suggestions the method becomes:
Inputting the cards of the players
This is the way you input the cards of the players and it's very strange:
With the helper methods developed in the earlier section,
all that code can be reduced to:
Next steps
Look for other opportunities to extract logic to more, smaller functions.
You should be able to gradually simplify and reduce the size of the code.
The most important thing to improve is to decompose this code to multiple smaller functions. At the same time, remove all code in the global namespace, so that the program has this structure:
import ...
def main():
...
if __name__ == '__main__':
main()See the next section for a closely related tip.
Don't repeat yourself
This snippet is repeated twice to input cards:
print "Input player {0}'s cards. Note: Capitals only!".format(player_one_name)
while temp != "END":
player_one.append(temp)
temp = raw_input("Card: ")
# Remove the blank in the array
if '' in player_one: player_one.remove('')The most common technique to reduce repeated logic is to use functions,
for example:
def input_cards(player, player_name):
print "Input player {0}'s cards. Note: Capitals only!".format(player_name)
while temp != "END":
player.append(temp)
temp = raw_input("Card: ")
# Remove the blank in the array
if '' in player:
player.remove('')Then you could call this with for each player:
input_cards(player_one, player_one_name)
input_cards(player_two, player_two_name)Improving the input of cards
The current method of inputting cards could use some improvement:
- Instead of removing blanks, it would be better to never add any
- Instead of converting input values to numbers later, it would be better at input time
- You should validate input here rather than later, so that the rest of the program can trust the input data without worrying about possibly invalid values
- The loop condition is evaluated pointlessly when it runs for the first time. It would be better to buy convert it to a
while Trueloop.
- The message "Note: Capitals only!" is unclear. I suppose you meant that users should enter "END" with all capitals to end the input
Applying the above suggestions the method becomes:
def input_cards(player, player_name):
print("Input player {0}'s cards. To stop, enter END with all capitals!".format(player_name))
while True:
temp = raw_input("Card: ")
if temp == "END":
break
try:
player.append(int(temp))
except ValueError:
print('Not a number: {0}'.format(temp))Inputting the cards of the players
This is the way you input the cards of the players and it's very strange:
# Input the players cards!
while i <= 1:
# Get Player one's cards
if i == 0:
# ...
i += 1
# Get Player two's cards
if i == 0:
# ...
i += 1With the helper methods developed in the earlier section,
all that code can be reduced to:
# Input the players cards!
input_cards(player_one, player_one_name)
input_cards(player_two, player_two_name)Next steps
Look for other opportunities to extract logic to more, smaller functions.
You should be able to gradually simplify and reduce the size of the code.
Code Snippets
import ...
def main():
...
if __name__ == '__main__':
main()print "Input player {0}'s cards. Note: Capitals only!".format(player_one_name)
while temp != "END":
player_one.append(temp)
temp = raw_input("Card: ")
# Remove the blank in the array
if '' in player_one: player_one.remove('')def input_cards(player, player_name):
print "Input player {0}'s cards. Note: Capitals only!".format(player_name)
while temp != "END":
player.append(temp)
temp = raw_input("Card: ")
# Remove the blank in the array
if '' in player:
player.remove('')input_cards(player_one, player_one_name)
input_cards(player_two, player_two_name)def input_cards(player, player_name):
print("Input player {0}'s cards. To stop, enter END with all capitals!".format(player_name))
while True:
temp = raw_input("Card: ")
if temp == "END":
break
try:
player.append(int(temp))
except ValueError:
print('Not a number: {0}'.format(temp))Context
StackExchange Code Review Q#115212, answer score: 6
Revisions (0)
No revisions yet.