2
\$\begingroup\$

I'm looking for tips on cleaning this up. I think it's legible, but it sure ain't pretty. In particular, I'd love to clean up the hitCheck function. I've dealt with switches before in other languages, so I was kind of stuck on trying simulating that. getReady looks messy to me, but I'm not sure how I'd go about tidying that up either.

import random
import time

def hitCheck(rng, acc):
    hitTotal = acc + rng
    if hitTotal > 190:
        print("CRIT! HEADSHOT!")
        return 5
    if 160 < hitTotal < 191:
        print("Lethal!")
        return 4
    if 120 < hitTotal < 161:
        print("Severe!")
        return 2
    if 80 < hitTotal < 121:
        print("Ouch!")
        return 1
    if 40 < hitTotal < 81:
        print("Grazed!")
        return 0.5
    if 0 < hitTotal < 41:
        print("Miss!")
        return 0

def firing(begin):
    print("BANG!")
    #end & start variables are used to tell how fast the user is pressing the key.
    #The faster they go, the more inaccurate their shots.
    end = time.time()
    accuracy = int((end - begin) * 60)
    if accuracy > 100:
        accuracy = 100
    #print("Accuracy: ", accuracy)
    #Not sure if this 'luck' generator is mathematically different
    #from just using 'int((random.random() * 100))'.
    luck1 = int((random.random() * 50))
    luck2 = int((random.random() * 50))
    luck = luck1 + luck2
    #print("Luck: ", luck)
    return hitCheck(luck, accuracy)

#Here's the main part of the app.
def getReady():
    running = False
    while not running:
        i = 0
        downed = 0
        total = 0
        while True:
            print("\n")
            total = input('Max attempts: ')
            print("\n")
            if total == '' or int(total) < 0:
                print("Value must be higher than 0!")
            else:
                total = int(total)
                break

        print("Fire away!")
        while i < total:
            #Other half to figuring out the accuracy.
            start = time.time()
            input()
            downed += firing(start)
            i += 1
            print(total - i, " shots left.")
            time.sleep(0.1)

        #Print the results!
        print("\n")
        print("Injuries:", int(downed))
        print("You felled", round(downed / 5), "foes!")
        print("\n")

        #Shall we continue?
        while True:
            running = input("More? yes / no: ")
            if running in ('no', 'n'):
                running = True
                break
            elif running in ('yes', 'y'):
                running = False
                break
            else:
                continue

getReady()
\$\endgroup\$
2
\$\begingroup\$

The way you normally emulate switches in Python is with a dictionary. That's a bit of a funny solution when you have to evaluate all these conditions though. If they were regular values then you could mathematically determine them and save lines that way, but since you have the odd variations those changes would only obfuscate your code for the sake of a cleverer seeming solution. I would suggest you gather the printed values into a result at least. You could access them with a value key and return that value the same way.

hit_strings = {
                5: "CRIT! HEADSHOT!"
                3: "Lethal!",
                2: "Severe!",
                1: "Ouch!",
                0.5: "Grazed!",
                0: "Miss!",
              }

def hitCheck(rng, acc):
    hitTotal = acc + rng
    if hitTotal > 190:
        result = 5
    if 160 < hitTotal < 191:
        result = 4
    if 120 < hitTotal < 161:
        result = 2
    if 80 < hitTotal < 121:
        result = 1
    if 40 < hitTotal < 81:
        result = 0.5
    if 0 < hitTotal < 41:
        result = 0

    print (hit_strings[result])
    return result

It's still long, but this does make it more modular and easier to expand with more values.

The luck generator isn't particular different no, just have one call. You should also remove your defunct comments that you used as print tests.

Your big problem in getReady is that you try to cram everything into one function. You could separate them out easily with functions like get_attempts

def get_attempts():
    while True:
        print("\n")
        total = input('Max attempts: ')
        print("\n")
        if total == '' or int(total) < 0:
            print("Value must be higher than 0!")
        else:
            total = int(total)
            return total

#Here's the main part of the app.
def getReady():
    running = False
    while not running:
        i = downed = total = 0
        total = get_attempts()

Though going back to get_attempts, you should just put newline characters in your input string instead of on separate lines. Also it's better to validate that input is actually a number. The most Pythonic way to do this is to attempt to turn it into an integer and catch it if that's not possible.

def get_attempts():
    while True:
        total = input('\n\nMax attempts:\n\n')
        try:
            total = int(total)
        except ValueError:
            print("Value must be a number!")

Then you can test if it's greater than 0 and print the other message if it's not.

        if total > 0:
            return total
        print("Value must be higher than 0!")

Splitting up your functions like this will make your code much neater and easier to change.

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

More functions, please

Each loop does an important job, and as such deserves a name. For example, asking for a continuation must be factored out into a function, e.g.

    def want_more():
        while True:
            answer = input('More: yes / no?')
            if answer in ('no, 'n'):
                return False
            if answer in ('yes', 'y'):
                return True
            print 'Only yes or no, please')

Luck generator is also a good candidate to be a function, especially since you are not sure how it should work (BTW, sum of two randoms is mathematically different from a single random twice that range).

Hit checking

Computation of hitTotal doesn't belong here. This is a utility function and shouldn't contain any game logic.

The second test is redundant in all cases. When you get to the 160 < hitTotal < 191 case you already know that hitTotal is less than 191.

Besides that, you may want to set up a list of triples (threshold, message, score) and iterate over it in a loop.

\$\endgroup\$
  • \$\begingroup\$ I'm not sure what you mean by "Computation of hitTotal doesn't belong here". Where else would it go? At the end of firing() and just pass along the total to hitCheck()? And I'm also unclear about "set up a list of triples". Do you happen to have an example of a triple in action that I could reference? \$\endgroup\$ – zomgdavidbowie Oct 8 '15 at 17:38
  • 1
    \$\begingroup\$ @zomgdavidbowie You got it correctly. Keep the game logic together. \$\endgroup\$ – vnp Oct 8 '15 at 17:39
1
\$\begingroup\$

Separate out the data from the control logic. In the future, you can add in new hit levels just by adding a row in the data, the code would be the same:

hit_matrix = [
    (40, "Miss!", 0),
    (80, "Grazed!", 0.5),
    (120, "Ouch!", 1),
    (160, "Severe!", 2)
    (190, "Lethal!", 4),
    (1000, "CRIT! HEADSHOT!", 5),
    ]
def hitCheck(rng, acc):
    hitTotal = acc + rng
    for max_value, msg, return_value in hit_matrix:
        if hitTotal <= max_value:
            break;
    print(msg)
    return return_value

Another area of code where you have an easy improvement:

accuracy = max(100, int((end - begin) * 60))
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Nice job on your first answer. \$\endgroup\$ – SirPython Oct 8 '15 at 22:55

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.