My second game: War Card game V.1

Question

I'm a beginner programmer that's really motivated at the moment. I made this in two days and I'm really proud of it because I never used any tutorials or any online help. Just good ol' grey matter and for real I think I did a much better job than my last game thanks to you guys! I really appreciate the feedback on my last code, it really helped me.

The game consists of having the largest card. So for example, if I have A and you have, say 9, you lose because A is "bigger" than 9 and so on. For physical game play you take one card at a time from a deck and the biggest card wins.

This is my code and it's working correctly.

import string
import random

# Main game Loop
def game():

    #Make the Cards
    def make_cards():

        cards = []
        suits = ["◆", "♥", "♠", "♣"]

        for suit in suits:
            for i in range(2, 11):

                card_id = str(i) + suit

                if i == 10:

                    cards.append(card_id +  " " + card_id + "\n\n" " tony " "\n\n" + card_id + " " + card_id + "\n" )

                else:
                    cards.append( card_id +  "   " + card_id + "\n\n" " tony " "\n\n" + card_id + "   " + card_id + "\n" )

        for suit in suits:
            for i in ["J","Q","K","A"]:
                card_id = i + suit
                cards.append( card_id +  "   " + card_id + "\n\n" + " tony " "\n\n" + card_id + "   " + card_id + "\n" )


        return cards

    cards = make_cards()

    # Distribute the cards

    def play_cards(cards):

        card_shuffle = [random.choice(cards) for i in cards]
        play_cards.p1 = card_shuffle[0:26]
        play_cards.p2 = card_shuffle[26:52]

        return play_cards.p1, play_cards.p2

    play_cards(cards)


    # Show cards in game
    def card_dump(input, p1, p2):

        if input == "":
            win_add()

            return (
                    print(game_logic()),
                    print("\n"),
                    print(" __________________________________"),
                    print("|        WIN COUNTER DELUXE        |"),
                    print("".join(win_add.p1)),
                    print("|__________________________________|"),
                    print("\n"),
                    print("          Player One Card\n"),
                    print(p1[0]),
                    print("\n"),
                    print(" __________________________________"),
                    print("|        WIN COUNTER DELUXE        |"),
                    print("".join(win_add.p2)),
                    print("|__________________________________|"),
                    print("\n"),
                    print("          Player Two Card\n"),
                    print(p2[0]),
                    play_cards.p1.pop(0),
                    play_cards.p2.pop(0)
                )

    who_won = []

    # Game logic

    def game_logic():

        p1 = play_cards.p1[0][:1]
        p2 = play_cards.p2[0][:1]

        letter_value = {"A": 13, "K":12, "Q":11, "J":10}


        if p1 == "1":
            p1 = "10"
        if p2 == "1":
            p2 = "10"


        if p1 == p2:
            who_won.append(0)


        elif p1.isdigit() == True and p2.isdigit() == True:

            if int(p1) > int(p2):
                who_won.append(1)
            else:
                who_won.append(2)

        elif p1.isdigit() == False and p2.isdigit() == False:

            if letter_value[p1] > letter_value[p2]:
                who_won.append(1)
            else:
                who_won.append(2)


        elif p1.isdigit() == True and p2.isdigit() == False:

            if int(p1) > int(letter_value[p2]):
                who_won.append(1)
            else:
                who_won.append(2)

        elif p1.isdigit() == False and p2.isdigit() == True:

            if int(p2) > int(letter_value[p1]):
                who_won.append(2)   
            else:
                who_won.append(1)

        return ""


    game_logic()

    # Return the list of how many times each player won

    def end_game():

        return who_won
    # Game score board "Win Counter Deluxe"
    def win_add():

        win_add.p1 = []
        win_add.p2 = []

        for i in who_won:

            if 1 == i:
                win_add.p1.append( " |")

            elif 2 == i:
                win_add.p2.append(" |")

        return win_add.p1, win_add.p2

    # Outcome Loop
    p1 = play_cards.p1
    p2 = play_cards.p2

    x = end_game()

    count = 0

    while True:

        if count == 26:

            p1_won = x.count(1)
            p2_won = x.count(2)
            draws = x.count(0)

            if p1_won == p2_won:
                print(f"The game finished in a DRAW. {p1_won} VS {p2_won}")
                break

            elif p1_won > p2_won:
                print(f"Player // ONE // won the game with {p1_won} wins VS {p2_won} for player // TWO //. There were {draws} draws.")
                break

            else:
                print(f"Player // TWO // won the game with {p2_won} wins VS {p1_won} wins for player // ONE //. There were {draws} draws.")
                break
        card_dump(input("Please hit enter"),p1, p2)
        count += 1



def main():
    game()
    while "y" in input("Play again? [Y/n]").lower():
        game()

if __name__ == '__main__':
    main()

Answer 1

Style

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide which will be very useful when it comes to writing a more Pythonic code.

• Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. An object's docstring is defined by including a string constant as the first statement in the object's definition. I see you wrote many comments above each of your functions and I suggest to include docstrings instead for these functions indicating what they do and what they return and type hints(if necessary when functions have many parameters).

  example:

  def make_cards():
      """Return deck of cards"""
      # do things
  

• Too many blank lines: according to PEP0008: Surround top-level function and class definitions with two blank lines.Method definitions inside a class are surrounded by a single blank line.Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

• Nested functions: since most of your functions do not have any parameters, there is no need for nested functions that complicate the code, unless your program has many aspects and needs many functions then you might consider using a class(which is not needed in your case). Nested functions are usually short and very specific in what they do (Usually they use the parameters of the enclosing function and do a specific task which is not the case here).

• Long lines: (lines 174, 178)

  print(f"Player // ONE // won the game with {p1_won} wins VS {p2_won} for player // TWO //. There were {draws} draws.") 
  

  According to PEP0008 a line should contain 79 characters max.

• Space around operators: card_dump(input("Please hit enter"),p1, p2) a space should be left on both sides of a binary operator(+-*/,=><|^&!=) for readability.

Code

From my understanding the game is all about a 2-player card virtual game that keeps displaying cards and calculates a score at the end. I think this code can be shortened, let's dissect your program parts:

def make_cards():
    # do stuff

Since no docstrings are included, I'm going to imply what they might be intended to do: this function creates 52 cards and returns a list with a very non-readable content.

a sample of what is returned :

['2◆ 2◆\n\n tony \n\n2◆ 2◆\n', '3◆ 3◆\n\n tony \n\n3◆ 3◆\n', '4◆ 4◆\n\n tony \n\n4◆ 4◆\n', '5◆ 5◆\n\n tony \n\n5◆ 5◆\n', '6◆ 6◆\n\n tony \n\n6◆]

This is a very hurting my eyes to read and this might be very annoying to debug (if not impossible), I suggest you create the deck in the following way:

def deck():
    """Return a list of 52-cards deck."""
    suits = '◆♥♠♣'
    digits = [str(number) for number in range(2, 11)]
    specials = 'AKQJ'
    special_cards = [special + suit for special in specials for suit in suits]
    numbered_cards = [number + suit for number in digits for suit in suits]
    return special_cards + numbered_cards 

returns: ['A◆', 'A♥', 'A♠', 'A♣', 'K◆', ...] which is much more readable and has the same use.

The play_cards() function: Does not return the full deck and if you want to check yourself, try running the following line:

print(len(set(play_cards(cards)[0])), len(set(play_cards(cards)[1])))

output: 21 22 (43 cards instead of 52) and it will return different results of course each time you try running it, so it does not even return the full shuffled deck.

To fix the problem I suggest you use random.shuffle()

random.shuffle(cards)

then the play_cards() function is unecessary and you can shuffle the cards before returning them in the make_cards() function (the one I called deck())

in the game_logic() function:

elif p1.isdigit() == False and p2.isdigit() == False:

this line repeated several times in different forms, here's the correct way of writing it:

if not p1.isdigit() and not p2.isdigit():

Here's an improved version of the code:

import random


def deck():
    """Return a list of 52-card deck."""
    suits = '◆♥♠♣'
    digits = [str(number) for number in range(2, 11)]
    specials = 'AKQJ'
    special_cards = [special + suit for special in specials for suit in suits]
    numbered_cards = [number + suit for number in digits for suit in suits]
    cards = special_cards + numbered_cards
    return cards


def get_winner(card1, card2):
    """Determine winner and return 1 or 2 or 0 for a tie."""
    suit_ranks = {'♣': 1, '◆': 2, '♥': 3, '♠': 4}
    special_ranks = {'J': 1, 'Q': 2, 'K': 3, 'A': 4}
    if card1 == card2:
        return 0
    if card1[0].isdecimal() and card2[0].isalpha():
        return 2
    if card1[0].isalpha() and card2[0].isdecimal():
        return 1
    if card1[0].isdecimal() and card2[0].isdecimal():
        if int(card1[0]) > int(card2[0]):
            return 1
        if int(card1[0]) < int(card2[0]):
            return 2
    if card1[0].isalpha() and card2[0].isalpha():
        if special_ranks[card1[0]] > special_ranks[card2[0]]:
            return 1
        if special_ranks[card1[0]] < special_ranks[card2[0]]:
            return 2
    if card1[-1] != card2[-1] and card1[:-1] == card2[:-1]:
        if suit_ranks[card1[-1]] > suit_ranks[card2[-1]]:
            return 1
        if suit_ranks[card1[-1]] < suit_ranks[card2[-1]]:
            return 2


def play_game():
    """Display rounds interactively and results at the end."""
    cards = deck()
    rounds = input('Enter the number of rounds to play: ')
    while not rounds.isdecimal():
        print('Invalid rounds number')
        rounds = input('Enter the number of rounds to play: ')
    games_played = 0
    player1_score, player2_score = 0, 0
    while games_played < int(rounds):
        confirm_round = input(f'Press enter to display round {games_played} or q to exit: ')
        while confirm_round and confirm_round != 'q':
            confirm_round = input(f'Press enter to display round {games_played} or q to exit: ')
        if confirm_round == 'q':
            print('Thank you for playing cards.')
            print(30 * '=')
            exit(0)
        player1_card = random.choice(cards)
        player2_card = random.choice(cards)
        print(f'player 1 card: {player1_card}')
        print(f'player 2 card: {player2_card}')
        winner = get_winner(player1_card, player2_card)
        if winner == 0:
            print('Tie!')
        if winner == 1:
            print('Player 1 wins.')
            player1_score += 1
        if winner == 2:
            print('Player 2 wins.')
            player2_score += 1
        games_played += 1
        print(30 * '=', '\n')
    print(30 * '=')
    print(f'Total rounds played: {games_played}')
    print(f'Player 1 {player1_score}-{player2_score} player 2')
    if player1_score > player2_score:
        print(f'Winner is Player 1 ({player1_score} out of {games_played} games played)')
    if player2_score > player1_score:
        print(f'Winner is Player 2 ({player2_score} out of {games_played} games played)')
    if player1_score == player2_score:
        print('Neither wins, TIE!')


if __name__ == '__main__':
    play_game()

Answer 2

You have a lot of places where you do something like

if int(p1) > int(p2):
    who_won.append(1)
else:
    who_won.append(2)

There's more duplication than necessary here. At the very least, you should move the call to append out so it's only called once. If you every change how who_won works, you don't want to have to change a ton of things all over. The less places you use it the easier it will be to refactor later. You can use a conditional expression here:

who_won.append(1 if int(p1) > int(p2) else 2)

---

You're comparing against True in a few places:

elif p1.isdigit() == True and p2.isdigit() == True:

This is unnecessary. if already interprets what you give it as either "truthy" or "falsey". == True is redundant. Just reduce it to:

elif p1.isdigit() and p2.isdigit():

That reads much more fluently anyways.

---

At the top, you have a giant chunk consisting of calls to print:

print(game_logic()),
print("\n"),
print(" __________________________________"),
print("|        WIN COUNTER DELUXE        |"),
. . .

Calling print excessively isn't a good idea, even though it won't really matter here. I would expect it to be more performant (and more readable) to use a single print with a sep="\n" argument passed:

print(game_logic(),
      "\n",
      " __________________________________",
      "|        WIN COUNTER DELUXE        |",
      . . .
      sep="\n")  # sep="\n" tells it to insert a newline between arguments

---

card_shuffle = [random.choice(cards) for i in cards]

This doesn't seem like a "shuffle". This will, unless I'm overlooking something, not return a list with the original proportion of cards. It will randomly have more of different cards than others. Just use random.shuffle:

random.shuffle(cards)  # Shuffles inplace instead of returning a new list.

If you wanted to avoid mutating the original, just make a copy first:

random.shuffle(cards[:])