Wedding Photography Cost Calculator

Question

This is my first code. I would really appreciate a critique for it, if anyone can spare the time.

def wedcost(hours):

    print " "

    try:
        hours = int(hours)
    except ValueError:
        print "Invalid input"
        new_hours = raw_input("Please use only numeric characters. ")
        hours = int(new_hours)
        print " "

    tcost = hours * 75 # $75/hr for two shooters
    if hours < 4:
        tcost = 250 # price starts at a fixed $250 from 1 to 3 hours
    if hours >= 6:
        tcost = hours * 100 # I don't like long shifts

    print "The total cost of our services will be $" + str(tcost) + "."

    print " "

    def recalc(answer): # this second function is where I had the most trouble

        outcome = 0 # only method I could get to work properly
        if answer.startswith('y' or 'Y'): 
            print " "
            outcome += 1
            wedcost(raw_input("How long will we be on location? "))
        if answer.startswith('n' or 'N'):
            print " "
            print "Have a nice day :)"
            outcome += 1
        if outcome == 0:
            print "Invalid input"
            recalc(raw_input("Would you like to recalculate? ('Yes' or 'No') "))

        # I had previously tried checking 'answer' to items in lists

    recalc(raw_input("Would you like to recalculate? ('Yes' or 'No') "))

wedcost(raw_input("How long will we be on location? "))

I'm sure that there's a better way to go about this task. Any suggestions or hints?

Answer 1

Well, this code has a few problems:

1. Typing non-numbers twice in a row causes a ValueError.

2. It is not quite PEP-8 compliant (the official python style guide)

3. No docstrings

4. Final bit of code should be wrapped in a if __name__ == "__main__": block to add reusability.

5. It is not very reusable

---

Now let's fix all that:

Wrap your input section in it's own functions like this:

def get_num(prompt, errorMsg = "I'm sorry, that's not a valid number"):
    """Returns a number from the user"""
    while True:
        try:
            input = int(raw_input(prompt))
        except ValueError:
            print errorMsg
        else:
            return input

def get_yes_no(prompt, errorMsg = "I'm sorry, I didn't catch that. Could you try again?"):
    """Returns True for yes, False for no"""
    while True:
        input = raw_input(prompt)
        if input.startswith('y' or 'Y'):
            return True
        elif input.startswith('n' or 'N'):
            return False
        print errorMsg

This allows you to have more robust and more readable code

Naming: function names should be lowercase and separated by _. So wedcost(hours) becomes wed_cost(hours).

Also your function doesn't do what is advertised, since it keeps on recalculating.

Let's wrap that into a main function like this:

def main():
    wed_cost(get_num("How long will we be on location? "))
    while get_yes_no("Would you like to recalculate? ('Yes' or 'No') "):
        wed_cost(get_num("How long will we be on location? "))
    print "Have a nice day :)"

Then we can add this little bit of code to the bottom:

if __name__ == "__main__":
    main()

Which will run your code if it is directly called, but also allows you to reuse this code elsewhere with an import without causing any code to be run.

---

Finally let's put all your magic numbers (such as 75, 100 etc) somewhere more recyclable by defining some constants at the top of the code:

MIN_FEE = 250
HOURLY_FEE = 75
OVERTIME_HOURS = 6
OVERTIME_FEE = 100

Then the wed_cost function becomes:

def wed_cost(hours):
    """Prints a message with the cost of a wedding shoot given a number of hours"""
    totalCost = 0
    if hours >= OVERTIME_HOURS:
        totalCost = hours * OVERTIME_FEE
    totalCost = max(hours * HOURLY_FEE, MIN_FEE)
    print "The total cost of our services will be ${}.".format(totalCost)

This makes your code more understandable, and more debuggable :)