4
\$\begingroup\$

I'm teaching myself Python via Zed Shaw's LPTHW and he suggested to me that I try to condense my code so I can call up bits of it within a function. Right now this is boggling my mind and I can't figure out a way to condense it that doesn't result in an error regarding global and local variables in reference to weapons. As it stands, this code works, but as you can see, it's long and gross.

### Shop / buy weapons room ###############

def buy_weapon(weapons):
    global gold
    """big bit of code that allows you to buy a weapons from a weapon list.
The function acts a little differently after level zero weapons"""
    global current_weapon
    if weapons == level_zero_weapons:
        sword_price = level_zero_price()
        blunt_price = level_zero_price()
        agile_price = level_zero_price()
        print t.bright_magenta_on_black + """
Please type in the weapon you want to buy.

%s, price: %d gold pieces

%s, price: %d gold pieces

%s, price: %d gold pieces.
""" % (weapons[0], sword_price, weapons[1], blunt_price,weapons[2], 
       agile_price)

        weapon_choice = raw_input(":> ")
        if weapons[0] in weapon_choice and gold >= sword_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - sword_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[0]
            inventory(weapons[0])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[1] in weapon_choice and gold >= blunt_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - blunt_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[1]
            inventory(weapons[1])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[2] in weapon_choice and gold >= agile_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - agile_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[2]
            inventory(weapons[2])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        else: 
            print "I dont know what %s means" % weapon_choice
            next()
            buy_weapon(level_zero_weapons)

    elif weapons == level_one_weapons:
        sword_price = level_one_price()
        blunt_price = level_one_price()
        agile_price = level_one_price()
        print"""
Type in the weapon you want to buy, type quit to return to the barracks.

%s, price: %d gold pieces

%s, price: %d gold pieces

%s, price: %d gold pieces.
""" % (weapons[0], sword_price, weapons[1], blunt_price, weapons[2],
       agile_price)

        weapon_choice = raw_input(":> ")
        if weapons[0] in weapon_choice and gold >= sword_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - sword_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[0]
            inventory(weapons[0])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[1] in weapon_choice and gold >= blunt_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - blunt_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[1]
            inventory(weapons[1])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[2] in weapon_choice and gold >= agile_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - agile_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[2]
            inventory(weapons[2])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif "quit" in weapon_choice:
            barracks()

        else: 
            print "Either you dont have enough money, or I dont know what %s means" % weapon_choice
            next()
            buy_weapon(level_one_weapons)

    elif weapons == level_two_weapons:
        sword_price = level_two_price()
        blunt_price = level_two_price()
        agile_price = level_two_price()
        print"""
Type in the weapon you want to buy, type quit to return to the barracks.

%s, price: %d gold pieces

%s, price: %d gold pieces

%s, price: %d gold pieces.
""" % (weapons[0], sword_price, weapons[1], blunt_price,weapons[2], 
       agile_price)

        weapon_choice = raw_input(":> ")
        if weapons[0] in weapon_choice and gold >= sword_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - sword_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[0]
            inventory(weapons[0])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[1] in weapon_choice and gold >= blunt_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - blunt_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[1]
            inventory(weapons[1])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[2] in weapon_choice and gold >= agile_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - agile_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[2]
            inventory(weapons[2])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif "quit" in weapon_choice:
            barracks()

        else: 
            print "Either you dont have enough money, or I dont know what %s means" % weapon_choice
            next()
            buy_weapon(level_two_weapons)

    elif weapons == level_three_weapons:
        sword_price = level_three_price()
        blunt_price = level_three_price()
        agile_price = level_three_price()
        print"""
Type in the weapon you want to buy, type quit to return to the barracks.

%s, price: %d gold pieces

%s, price: %d gold pieces

%s, price: %d gold pieces.
""" % (weapons[0], sword_price, weapons[1], blunt_price,weapons[2], 
       agile_price)

        weapon_choice = raw_input(":> ")
        if weapons[0] in weapon_choice and gold >= sword_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - sword_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[0]
            inventory(weapons[0])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[1] in weapon_choice and gold >= blunt_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - blunt_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[1]
            inventory(weapons[1])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[2] in weapon_choice and gold >= agile_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - agile_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[2]
            inventory(weapons[2])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif "quit" in weapon_choice:
            barracks()

        else: 
            print "Either you dont have enough money, or I dont know what %s means" % weapon_choice
            next()
            buy_weapon(level_three_weapons)

    elif weapons == level_four_weapons:
        sword_price = level_four_price()
        blunt_price = level_four_price()
        agile_price = level_four_price()
        print"""
Type in the weapon you want to buy, type quit to return to the barracks.

%s, price: %d gold pieces

%s, price: %d gold pieces

%s, price: %d gold pieces.
""" % (weapons[0], sword_price, weapons[1], blunt_price,weapons[2], 
       agile_price)

        weapon_choice = raw_input(":> ")
        if weapons[0] in weapon_choice and gold >= sword_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - sword_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[0]
            inventory(weapons[0])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[1] in weapon_choice and gold >= blunt_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - blunt_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[1]
            inventory(weapons[1])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif weapons[2] in weapon_choice and gold >= agile_price:
            character_sheet.remove(character_sheet[9])
            gold = gold - agile_price
            character_sheet.insert(9, "You have %s gold pieces." % gold) 
            current_weapon = weapons[2]
            inventory(weapons[2])
            character_sheet.append("Current Weapon: %s" % current_weapon)
            barracks()

        elif "quit" in weapon_choice:
            barracks()

        else: 
            print "Either you dont have enough money, or I dont know what %s means" % weapon_choice
            next()
            buy_weapon(level_four_weapons)      
    else:
        print"~~~There is a bug somwhere, forgot to assign (weapons)\n\n\n"
    raw_input(t.white_on_red("""
Your current weapon is now a %s. Press Enter To Continue
""" % current_weapon))
\$\endgroup\$
3
  • 2
    \$\begingroup\$ You've certainly noticed the amount of duplication in your. Start by looking at similar pieces of code, extracting what is variable and writing functions for those bits with the variable aspects as parameters of the function. \$\endgroup\$
    – Pedro Romano
    Commented Oct 7, 2012 at 18:21
  • \$\begingroup\$ It seems as though the logical variable that would be in a paramater would be weapons but when i try to use that, i got global related errors \$\endgroup\$
    – lerugray
    Commented Oct 7, 2012 at 22:28
  • \$\begingroup\$ After implementing the changes proposed in @GlennRogers excellent answer, the next step is getting rid of those globals! If you want to keep it simple for now and don't go object oriented, you can use for example a dict to save your state, replacing the multiple global variables and pass that around as a function parameter. \$\endgroup\$
    – Pedro Romano
    Commented Oct 8, 2012 at 23:43

1 Answer 1

Reset to default
2
\$\begingroup\$

As you've discovered, globals are hard to deal with.

Assuming that you now know what to do with your repeated code, here's one method of dealing with them.

If we start with a function:

INV_GOLD = 9

def buy_offered_weapon( offered_weapon, price ):
    global gold, current_weapon
    character_sheet.remove( character_sheet[INV_GOLD] )
    gold = gold - price
    character_sheet.insert( INV_GOLD, "You have %s gold pieces." % gold ) 
    current_weapon = offered_weapon
    add_to_inventory( offered_weapon )
    character_sheet.append( "Current Weapon: %s" % current_weapon )
    back_to_barracks()

Remove any global line and mark out which variables are neither local to the function, nor passed in as a parameter (this is obviously easier to do with smallish functions!).

def buy_offered_weapon( offered_weapon, price ):
    GLOBAL_character_sheet.remove( GLOBAL_character_sheet[INV_GOLD] )
    GLOBAL_gold = GLOBAL_gold - price
    GLOBAL_character_sheet.insert( INV_GOLD, "You have %s gold pieces." % GLOBAL_gold ) 
    GLOBAL_current_weapon = offered_weapon
    add_to_inventory( offered_weapon )
    GLOBAL_character_sheet.append( "Current Weapon: %s" % GLOBAL_current_weapon )
    back_to_barracks()

Notice any patterns in the GLOBAL parts, in the same way as you simplified your buy/sell function. Separate these out into "access functions" for the globals - in this way, you isolate the globals into a (hopefully) single point, and remove their (direct) use scattered elsewhere in the code.

These access functions should be as small as possible.

def adjust_inventory_gold( gold_change ):
    GLOBAL_character_sheet.remove( GLOBAL_character_sheet[INV_GOLD] )
    GLOBAL_gold = GLOBAL_gold + gold_change
    GLOBAL_character_sheet.insert( INV_GOLD, "You have %s gold pieces." % GLOBAL_gold ) 

def set_current_weapon( weapon ):
    GLOBAL_current_weapon = weapon
    GLOBAL_character_sheet.append( "Current Weapon: %s" % GLOBAL_current_weapon )

Add back in the global statements as required:

def adjust_inventory_gold( gold_change ):
    global gold, character_sheet
    character_sheet.remove( character_sheet[INV_GOLD] )
    gold = gold + gold_change
    character_sheet.insert( INV_GOLD, "You have %s gold pieces." % gold )

def set_current_weapon( weapon ):
    global current_weapon, character_sheet
    current_weapon = weapon
    character_sheet.append( "Current Weapon: %s" % current_weapon )

Note that global character_sheet isn't strictly required (you're not assigning to it), but it's useful making it explicit as documentation.

def buy_offered_weapon( offered_weapon, price ):
    adjust_inventory_gold( -price )
    add_to_inventory( offered_weapon )
    set_current_weapon( offered_weapon )
    back_to_barracks()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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