3
\$\begingroup\$

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 < n:
            winner.append(deck[0])
            deck.remove(deck[0])
            i = i + 1

    def playcards(self, player1, player2, deck):
        if random.randint(1,2) == 1:
            deck.append(player2[0])
            deck.append(player1[0])
        else:
            deck.append(player1[0])
            deck.append(player2[0])

    def comparecards(self, deck):
        if deck[0] > deck[1]:
            return 0
        if deck[0] < deck[1]:
            return 2
        if deck[0] == deck[1]:
            return 1

    def create_deck(self):
        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:
                deck.append(Card(x, i, num))
                num = num + 1
            num = 2  
        return sorted(deck, key=lambda k: random.random())

    def deal_cards(self, deck):  
        x = 0
        i = 0
        player1 = []
        player2 = []
        while i < len(deck):
            if x == 0:
                player1.append(deck[i])
                x = 1
            else:
                player2.append(deck[i])
                x = 0
            i += 1
        return player1, player2

    def game(self):
        pl1wins = 0
        pl2wins = 0
        turnspl1 = []
        turnspl2 = []
        while (pl1wins + pl2wins) < 100: 
            deck = self.create_deck()
            players = self.deal_cards(deck)
            player1 = players[0]
            player2 = players[1]

            player1 = sorted(player1, key=lambda card: card.value, reverse = True)
            war = []

            turns = 0
            while len(player1) > 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 player1[0].value > player2[0].value:
                            self.playcards(player1, player2, war)
                            self.wincards(player1, war)

                        elif player1[0].value < player2[0].value:
                            self.playcards(player1, player2, war)
                            self.wincards(player2, war)

            if len(player1) != 0:
                pl1wins = pl1wins + 1
                turnspl1.append(turns)
            elif len(player2) != 0:
                pl2wins = pl2wins + 1
                turnspl2.append(turns)
        print str(pl1wins) + " versus " + str(pl2wins)
        print str(average(turnspl1)) + " versus " + str(average(turnspl2))
        print str(average(turnspl1 + turnspl2))

def average(array):
    a = 0
    for i in array:
        a = a + i

    average = a/len(array)
    return average

warGame = War()    
warGame.game()
\$\endgroup\$
3
\$\begingroup\$

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:
            # ...
\$\endgroup\$
2
\$\begingroup\$

Proper class inheritance

In any version of Python 2.x, all classes need to explicitly inherit from object, like this:

class MyClass(object):
    ...

If you do use Python 3.x, you don't need to explicitly inherit from object. For more info on this, see this Stackoverflow question.


str.format rather than concatenation

Consider the following line:

print str(pl1wins) + " versus " + str(pl2wins)

We can use str.format rather than concatenation to get rid of the calls to str, and make this clearer and easier to read.

print "{0} versus {1}".format(pl1wins, pl2wins)

Simplification

The following statement:

turns = turns + 1

Can be simplified using the addition assignment operator, +=, like this:

turns += 1

This also applies to other operators, like mathematical, and binary.

There are a few places where you're chaining if/elif/else statements. You can use a dictionary instead, like this:

options = {
    1: some_function
    ...
}

if some_input in options:
    options[some_input]()
else:
    ...

Miscellaneous nitpicks

I only have on nitpick, and that's the addition of if __name__ == "__main__": to the end, like this.

if __name__ == "__main__":
    war_game = War()
    war_game.game()

See this Stackoverflow question for more details.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.