3
\$\begingroup\$

I have been working on a small game called apple picker, it's text-based, and relvolves around picking and selling apples. Unfortunately I've been using global variables for this whole thing and it's starting to get a bit ugly looking. Is there a more efficient way to do this than using global variables?

from random import randint
import console, time

global gold
global apples
apples = 1
gold = 1

prompt = "> "

def main():
    global gold
    global apples
    console.clear()
    print "Gold: %r Apples: %r" % (gold, apples)
    print "Pick an apple?"
    choice = raw_input(prompt)
    if choice == "yes":
        pick()
    elif choice == "no":
        console.clear()
        global gold 
        global apples
        print "Apples: %r Gold: %r" % (apples, gold)
        print "Sell apples?"
        sell = raw_input(prompt)
        if sell == "yes" and apples >= 1:
            global gold
            global apples
            apple_sales = randint(1,5)
            gold = (gold * (apples / 2)) * apple_sales
            apples = apples - apples
            if gold >= 25 ** 25:
                console.clear()
                print "\t\t\tYou won!"
                print "Congrats on controlling the apple market!"
            else: 
                main()
        elif sell == "yes" and apples <= 0:
            print "\nNot enough apples!"
            time.sleep(0.7)
            main()
        elif sell == "no":
            main()
        else: 
            main()
    elif choice == "exit":
        console.clear()
        print "Bye..."
    else: 
        main()

def pick():
    console.clear()
    global gold
    global apples
    print "Type 0 to exit. How many?"
    print "Apples: %r Gold %r" % (apples, gold)
    try: 
        apple_num = int(raw_input(prompt))
        if apple_num == 3 or apple_num == 2 or apple_num == 1:
            global apples
            apples = apples + apple_num
            time.sleep(0.5)
            pick()
        elif apple_num >= 4:
            console.clear()
            print "You can't haul that many apples!"
            time.sleep(0.5)
            pick()
        elif apple_num == 0:
            main()
    except ValueError:
        pick()

main()

Ignore the console module. I wrote this script on IOS.

\$\endgroup\$
0
1
\$\begingroup\$

Store the game data in a class and pass it around. Makes testing individual sections of the game easier.

I simplified some things here or there where I did not understand the logic.

# import console
from random import randint
import time


class EndGame(Exception):
    pass


class GameData(object):
    apples = 1
    gold = 1

    def __str__(self):
        return "Apples: %d Gold: %d" % (self.apples, self.gold)


def clear():
    # console.clear()
    pass


def delay(length):
    time.sleep(length)


def prompt(message):
    show(message)
    choice = raw_input("> ")
    if choice == "exit":
        raise EndGame
    return choice


def show(message):
    if isinstance(message, str):
        print message
    elif isinstance(message, (list, tuple)):
        print "\n".join(message)


def offer_alternatives(data):
    clear()
    show(str(data))

    choice = prompt("Sell apples?")
    if choice == "yes":
        if data.apples >= 1:
            apple_sales = randint(1, data.apples)
            data.gold += apple_sales  # not sure what the old logic was doing
            data.apples -= apple_sales
        elif data.apples <= 0:
            show("Not enough apples!")
            delay(0.7)

    return data


def run_game(data):
    keep_going = True

    try:
        while keep_going:
            show(str(data))

            choice = prompt("Pick an apple?")
            if choice == "yes":
                data = pick(data)
            else:
                data = offer_alternatives(data)

            if data.gold >= 25: # 25 ** 25 means 25 raised to the 25.
                clear()
                show(["You won!",
                      "Congrats on controlling the apple market!"])
                keep_going = False
    except EndGame:
        show("Bye")


def force_valid_choice(message, type, validator):
    value = None 

    while True:
        choice = prompt(message)
        try:
            value = type(choice)
        except ValueError:
            pass  
        else:
            if validator(value):
                break

    return value


def pick(data):
    clear()

    show(str(data))

    while True:
        choice = force_valid_choice("Type 0 to exit. How many?", int,
                                    lambda x: x >= 0)
        if choice >= 4:
            clear()
            show("You can't haul that many apples!")
            delay(0.5)
        elif choice == 0:
            return data
        elif choice in (3, 2, 1):
            data.apples += choice
            delay(0.5)
            return data


def main():
    data = GameData()
    run_game(data)


if __name__ == "__main__":
    main()
\$\endgroup\$
5
  • \$\begingroup\$ Why do you have to do if __name__ == "__main__": when you could just do main()? \$\endgroup\$ – Vladimir Putin Jun 24 '14 at 2:40
  • \$\begingroup\$ In the directory where this file is saved you can load it directly in the Python interpreter and experiment with each function. The trick is if you load it as a module no code is run. The 'name' will be 'filename.py' or the like. But if you run the program as "python filename.py" the 'name' is now main and the code under the if is run. \$\endgroup\$ – Sean Perry Jun 24 '14 at 4:49
  • \$\begingroup\$ Experimenting with the code in the interpreter is a great way to learn and develop code faster. Moving the main under the if also makes it so unittests can be run. \$\endgroup\$ – Sean Perry Jun 24 '14 at 4:49
  • \$\begingroup\$ @SeanPerry note that your GameData attributes are class attributes - it's OK in this case, where there will only be one instance at a time, but the user will notice some odd behaviour if, for example, a replay option is added. \$\endgroup\$ – jonrsharpe Jun 24 '14 at 10:48
  • \$\begingroup\$ Agreed @jonrsharpe. I was trying to keep it simple to match the OPs experience in Python. \$\endgroup\$ – Sean Perry Jun 24 '14 at 15:55
1
\$\begingroup\$

You don't need to list those as globals. Variables defined at the module-level namespace are available in functions in that module. Just remove the global declarations and you should be fine.

Another thing: calling main() is kind of dangerous. It's always better to do this instead:

if __name__ == '__main__':
  main()

That way main() won't be run when the module is imported.

\$\endgroup\$
2
  • \$\begingroup\$ I could also just do from file import main. Right? \$\endgroup\$ – Vladimir Putin Jun 24 '14 at 13:50
  • \$\begingroup\$ You could use from __file__ import main, but that violates the Zen of Python, particularly: "Explicit is better than implicit." \$\endgroup\$ – whereswalden Jun 24 '14 at 15:13
1
\$\begingroup\$

Couldn't send this answer yesterday because the site was down for some reason.

First thing you can fix is to add a guard before calling main(). This is the usual way to do things in Python so that you can import you files without running the code corresponding to main :

if __name__ == "__main__":
    main()

Then, one can see that you are using recursion to emulate loops. It is not quite a good idea in Python because recursion is not that optimised (for various reasons) so you should stick to loops if it doesn't make things more complicated. In your case, it doesn't at all : you can remove calls to main from your main function which becomes :

def main():
    global gold
    global apples
    while True:
        print "Gold: %r Apples: %r" % (gold, apples)
        print "Pick an apple?"
        choice = raw_input(prompt)
        if choice == "yes":
            pick()
        elif choice == "no":
            global gold
            global apples
            print "Apples: %r Gold: %r" % (apples, gold)
            print "Sell apples?"
            sell = raw_input(prompt)
            if sell == "yes" and apples >= 1:
                global gold
                global apples
                apple_sales = randint(1,5)
                gold = (gold * (apples / 2)) * apple_sales
                apples = apples - apples
                if gold >= 25 ** 25:
                    print "\t\t\tYou won!"
                    print "Congrats on controlling the apple market!"
                    return
            elif sell == "yes" and apples <= 0:
                print "\nNot enough apples!"
                time.sleep(0.7)
        elif choice == "exit":
            print "Bye..."
            return

From the pick function, you probably don't need to call main as the loop from the main function should handle things properly.

def pick():
    global gold
    global apples
    print "Type 0 to exit. How many?"
    print "Apples: %r Gold %r" % (apples, gold)
    try:
        apple_num = int(raw_input(prompt))
        if apple_num == 3 or apple_num == 2 or apple_num == 1:
            global apples
            apples = apples + apple_num
            time.sleep(0.5)
            pick()
        elif apple_num >= 4:
            print "You can't haul that many apples!"
            time.sleep(0.5)
            pick()
    except ValueError:
        pick()

We can apply the same kind of arguments to pick calling itself :

def pick():
    global gold
    global apples
    while True:
        print "Type 0 to exit. How many?"
        print "Apples: %r Gold %r" % (apples, gold)
        try:
            apple_num = int(raw_input(prompt))
            if apple_num == 3 or apple_num == 2 or apple_num == 1:
                global apples
                apples = apples + apple_num
                time.sleep(0.5)
            elif apple_num >= 4:
                print "You can't haul that many apples!"
                time.sleep(0.5)
            else:
                return
        except ValueError:
            pass

Then, you do not need to repeat global my_var before each and every use, you can do it once at the beginning of the function and that should be enough.

Also, if apple_num == 3 or apple_num == 2 or apple_num == 1 can be written in a concise way in Python : if apple_num in [3, 2, 1]. But as we are using integers, we can use a even better way :if 1 <= apple_num <= 3`.

Then, you can write apples += apple_nums instead of apples = apples + apple_num.

You can simply write apples = 0 instead of apples = apples - apples.

You should try to avoid writing the same condition twice, it makes things harder to understand/maintain. For instance, you could write :

        sell = raw_input(prompt)
        if sell == "yes":
            if apples >= 1:
                apple_sales = randint(1,5)
                gold = (gold * (apples / 2)) * apple_sales
                apples = 0
                if gold >= 25 ** 25:
                    print "\t\t\tYou won!"
                    print "Congrats on controlling the apple market!"
                    return
            elif apples <= 0:
                print "\nNot enough apples!"
                time.sleep(0.7)

and then, becomes apples is known to be an integer at all time, the second condition will always be true if the first is false : we don't need it. Last details on this : the pythonic way to write if my_int != 0 is if my_int. It applies to apples because it will be a positive-or-null integer. Thus, you can write : if apples

Now, we can actually go into your global issue : your pick function could return the number of picked apple and shouldn't print about the total number of apples and gold.

def main():
    apples = 1
    gold = 1
    while True:
        print "Gold: %r Apples: %r" % (gold, apples)
        print "Pick an apple?"
        choice = raw_input(prompt)
        if choice == "yes":
            apples += pick()
        elif choice == "no":
            print "Apples: %r Gold: %r" % (apples, gold)
            print "Sell apples?"
            sell = raw_input(prompt)
            if sell == "yes":
                if apples:
                    apple_sales = randint(1,5)
                    gold = (gold * (apples / 2)) * apple_sales
                    apples = 0
                    if gold >= 25 ** 25:
                        print "\t\t\tYou won!"
                        print "Congrats on controlling the apple market!"
                        return
                else:
                    print "\nNot enough apples!"
                    time.sleep(0.7)
        elif choice == "exit":
            print "Bye..."
            return

def pick():
    picked_apples = 0
    while True:
        print "Type 0 to exit. How many?"
        print "Picked apples: %r" % (picked_apples)
        try:
            apple_num = int(raw_input(prompt))
            if 1 <= apple_num <= 3:
                picked_apples += apple_num
                time.sleep(0.5)
            elif apple_num >= 4:
                print "You can't haul that many apples!"
                time.sleep(0.5)
            else:
                return picked_apples
        except ValueError:
            pass

Finally, you could make your user interface better by providing the different options:

def main():
    apples = 1
    gold = 1
    while True:
        print "Gold: %r Apples: %r" % (gold, apples)
        print "Action ? (pick/sell/exit)"
        choice = raw_input(prompt)
        if choice == "pick":
            apples += pick()
        elif choice == "sell":
            if apples:
                apple_sales = randint(1,5)
                gold = (gold * (apples / 2)) * apple_sales
                apples = 0
                if gold >= 25 ** 25:
                    print "\t\t\tYou won!"
                    print "Congrats on controlling the apple market!"
                    return
            else:
                print "\nNot enough apples!"
                time.sleep(0.7)
        elif choice == "exit":
            print "Bye..."
            return
\$\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.