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

Console weapons shop for adventure game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
adventuregameforshopweaponsconsole

Problem

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:

Solution

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()

Code Snippets

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()
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()
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 )
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 )
def buy_offered_weapon( offered_weapon, price ):
    adjust_inventory_gold( -price )
    add_to_inventory( offered_weapon )
    set_current_weapon( offered_weapon )
    back_to_barracks()

Context

StackExchange Code Review Q#16304, answer score: 2

Revisions (0)

No revisions yet.