# Wedding Photography Cost Calculator

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? • # only method I could get to work properly -- Is this code functional? – nhgrif Aug 2 '14 at 1:07 • @nhgrif Yes. Well, it works when I run it in IDLE. – muad-dweeb Aug 2 '14 at 1:23 • Hi, and welcome to Code Review, and congrats on breezing through the first-post review queue. – rolfl Aug 2 '14 at 1:56 • To me the big problem with this is that it would just be so much more convenient to use as a half dozen (or so) cells in a spreadsheet. – Jerry Coffin Aug 2 '14 at 3:38 • @JerryCoffin Correct. The fundamental purpose of this function was to allow me to figure out a code all on my own, without being hand-held or guided by a lesson plan. I figured that I would ask this community for critique if I could get it to function somewhat decently first. I really don't have any bigger plans for it, but I will be thoroughly exploring the feedback I have received. Better to catch bad practices when they're still fresh ;) – muad-dweeb Aug 2 '14 at 3:52 ## 1 Answer 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 :)

• Thank you so much. That is an unexpectedly detailed and helpful response. Cheers! – muad-dweeb Aug 2 '14 at 3:45