HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

War playing cards based game

Submitted by: @import:stackexchange-codereview··
0
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

Solution

Looping over lists

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 += 1


This 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 = 0


More 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 + 1


A 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 1


Naming

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 += 1
for card in deck:
        if x == 0:
            player1.append(card)
            x = 1
        else:
            player2.append(card)
            x = 0
def wincards(self, winner, deck):
    i = 0
    n = len(deck)
    while i < n:
        winner.append(deck[0])
        deck.remove(deck[0])
        i = i + 1
def 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.