patternpythonMinor
War playing cards based game
Viewed 0 times
playingcardswargamebased
Problem
How can I make this "War" card game code more visibly pleasing?
```
import random
import sys
class Card:
def __init__(self, rank, suit, value):
self.rank = rank
self.suit = suit
self.value = value
self.name = str(self.rank) + " of " + self.suit
class War:
def wincards(self, winner, deck):
i = 0
n = len(deck)
while i deck[1]:
return 0
if deck[0] 0 and len(player2) > 0:
turns = turns + 1
if (len(player1) + len(player2)) > 52:
print "Oh NO!"
sys.exit()
war = []
self.playcards(player1, player2, war)
player1.remove(player1[0])
player2.remove(player2[0])
result = self.comparecards(war)
if result == 0:
warGame.wincards(player1, war)
elif result == 2:
warGame.wincards(player2, war)
elif result == 1:
if len(player1) == 0:
player1.append(war[0])
war.remove(war[0])
if player1[0].value == player2[0].value:
player1[0].value = 0
if len(player2) == 0:
player2.append(war[1])
war.remove(war[1])
if player1[0].value == player2[0].value:
player2[0].value = 0
while len(war) > 0:
i = 0
for i in range(3):
if len(player1) > 1:
war.append(player1[0])
player1.remove(player1[0])
if len(player2) > 1:
war.append(player2[0])
player2.remove(player2[0])
if pla
```
import random
import sys
class Card:
def __init__(self, rank, suit, value):
self.rank = rank
self.suit = suit
self.value = value
self.name = str(self.rank) + " of " + self.suit
class War:
def wincards(self, winner, deck):
i = 0
n = len(deck)
while i deck[1]:
return 0
if deck[0] 0 and len(player2) > 0:
turns = turns + 1
if (len(player1) + len(player2)) > 52:
print "Oh NO!"
sys.exit()
war = []
self.playcards(player1, player2, war)
player1.remove(player1[0])
player2.remove(player2[0])
result = self.comparecards(war)
if result == 0:
warGame.wincards(player1, war)
elif result == 2:
warGame.wincards(player2, war)
elif result == 1:
if len(player1) == 0:
player1.append(war[0])
war.remove(war[0])
if player1[0].value == player2[0].value:
player1[0].value = 0
if len(player2) == 0:
player2.append(war[1])
war.remove(war[1])
if player1[0].value == player2[0].value:
player2[0].value = 0
while len(war) > 0:
i = 0
for i in range(3):
if len(player1) > 1:
war.append(player1[0])
player1.remove(player1[0])
if len(player2) > 1:
war.append(player2[0])
player2.remove(player2[0])
if pla
Solution
Looping over lists
When looping over elements of a collection,
if you don't really need the loop index,
then use an iterating
This accomplishes the same, but cleaner and more natural:
More simplifications
On top of @Ethan's review, some further simplifications are possible.
This method appends the content of
and empties the
A simpler way to accomplish the same thing:
In your implementation of
You could reduce your implementation to one line:
In this sequence of conditions,
if the first two conditions are both
then inevitably
so you don't need the last
Naming
Many of the variable names are not great.
For example in this bit:
It's often natural to give lists plural names,
such as "suits" and "ranks".
(On the other hand, "deck" is fine as it is, because it already implies a collection of cards.)
Then, these better names will also enable you to use better names than "i" and "x" when you loop:
When looping over elements of a collection,
if you don't really need the loop index,
then use an iterating
for loop instead of a while loop:i = 0
while i < len(deck):
if x == 0:
player1.append(deck[i])
x = 1
else:
player2.append(deck[i])
x = 0
i += 1This accomplishes the same, but cleaner and more natural:
for card in deck:
if x == 0:
player1.append(card)
x = 1
else:
player2.append(card)
x = 0More simplifications
On top of @Ethan's review, some further simplifications are possible.
This method appends the content of
deck at the end of winner,and empties the
deck:def wincards(self, winner, deck):
i = 0
n = len(deck)
while i < n:
winner.append(deck[0])
deck.remove(deck[0])
i = i + 1A simpler way to accomplish the same thing:
def wincards(winner, deck):
winner.extend(deck)
deck[:] = []In your implementation of
average, you reinvented the wheel by implementing sum, which exists as a built-in function in Python.You could reduce your implementation to one line:
def average(array):
return sum(array) / len(array)In this sequence of conditions,
if the first two conditions are both
False,then inevitably
deck[0] == deck[1],so you don't need the last
if statement and can return 1 directly:if deck[0] > deck[1]:
return 0
if deck[0] < deck[1]:
return 2
if deck[0] == deck[1]:
return 1Naming
Many of the variable names are not great.
For example in this bit:
suit = ['Spades', 'Hearts', 'Clubs', 'Diamonds']
rank = ['2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King', 'Ace']
deck = []
num = 2
for i in suit:
for x in rank:
# ...It's often natural to give lists plural names,
such as "suits" and "ranks".
(On the other hand, "deck" is fine as it is, because it already implies a collection of cards.)
Then, these better names will also enable you to use better names than "i" and "x" when you loop:
for suit in suits:
for rank in ranks:
# ...Code Snippets
i = 0
while i < len(deck):
if x == 0:
player1.append(deck[i])
x = 1
else:
player2.append(deck[i])
x = 0
i += 1for card in deck:
if x == 0:
player1.append(card)
x = 1
else:
player2.append(card)
x = 0def wincards(self, winner, deck):
i = 0
n = len(deck)
while i < n:
winner.append(deck[0])
deck.remove(deck[0])
i = i + 1def wincards(winner, deck):
winner.extend(deck)
deck[:] = []def average(array):
return sum(array) / len(array)Context
StackExchange Code Review Q#98473, answer score: 3
Revisions (0)
No revisions yet.