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

War Card Game Simulator

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

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 True loop.



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


With 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.