2
\$\begingroup\$

Is there a way to make this more pythonic?

def money():
    current_salary = float(input("What is your current salary? "))
    years = int(input("How many years would you like to look ahead? "))
    amount_of_raise = float(input("What is the average percentage raise you think you will get? ")) * 0.01

    for years in range(1, years + 1):
        current_salary += current_salary * amount_of_raise
        print('Looks like you will be making', current_salary,' in ', years,'years.')
money()
\$\endgroup\$
4
\$\begingroup\$
  • As others have noted, you'll probably want to do some error-checking on your inputs, which means writing a helper function to handle your prompts.
  • You're writing a program that handles money and using binary floats, which are designed for scientific calculations. You are likely to run into a variety of small calculation errors! Python has a decimal module for calculations involving money. This will also make your input-handling easier.
  • Don't reuse "years" as the iterator in the for loop when you already have a variable with that name. It works here, but only by luck.
  • Prefer the format() method to straight concatenation. In this case it allows you to easily print your numbers with comma separators.

For example:

from decimal import *    

def get_decimal(prompt):
    while True:
        try:
            answer = Decimal(input(prompt + " "))
            return answer
        except InvalidOperation:
            print("Please input a number!")

def money():
    current_salary = get_decimal("What is your current salary?")
    years = int(get_decimal("How many years would you like to look ahead?"))
    percent_raise = get_decimal("What is the average percentage raise you think you will get?") * Decimal("0.01")
    for year in range(1, years+1):
        current_salary += percent_raise * current_salary
        line = "Looks like you will be making ${0:,.2f} in {1} years.".format(current_salary, year)
        print(line)
\$\endgroup\$
  • \$\begingroup\$ from <module> import * is discouraged because it can cause namespace collisions and gives static analyzers a hard time determining where a name originates from. from decimal import Decimal is better. \$\endgroup\$ – Jace Browning Jul 8 '13 at 16:41
3
\$\begingroup\$

I would use a docstring and split the method to even smaller ones

def money():
    """Print Expectations on Salary"""
    salary, delta, perc_raise =  get_user_input()
    expectations = calculate_expectations(salary, delta, perc_raise)
    print_expectations(expectations)

money()

And I would store the expectations in a List

def calculate_expectations(salary, delta_max, perc_raise):
    """Calculate Expectations on Salary"""
    [salary*(1+perc_raise/100)**delta for delta in range(1, delta_max + 1)]
\$\endgroup\$
  • \$\begingroup\$ @codesparcle Thank you for the hint. I've edited my answer and renamed the variable to perc_raise. \$\endgroup\$ – ovhaag Jul 9 '13 at 10:09
2
\$\begingroup\$

Not sure if that it would make things more pythonic but here are 2 comments to make things less awkward.

You might want to rename your variable name in for years in range(1, years + 1): for something like for year in range(1, years + 1):.

It might make sense to use an additional variable to make things slighly clearer in your calculations:

def money():
    # TODO : Add error handling on next 3 lines
    current_salary = float(input("What is your current salary? "))
    years = int(input("How many years would you like to look ahead? "))
    amount_of_raise = float(input("What is the average percentage raise you think you will get? "))
    coeff = 1 + amount_of_raise * 0.01

    for year in range(1, years + 1):
        current_salary *= coeff
        print('Looks like you will be making', current_salary,' in ', year,'years.')
money()
\$\endgroup\$
2
\$\begingroup\$

Maybe you could write:

Instead of print('Looks like you will be making', current_salary,' in ', years,'years.') you could write print('Looks like you will be making %d in %d years.') % (current_salary, years)

Also, and this one is kind of important, you should check the input before converting it to an int or float (maybe the user goes crazy and throws a string just for the fun of it). You could maybe do that with a try: ... except: block.

And just to be nitpicky, what's your position on quotes? Do you use single or you use double?

\$\endgroup\$
  • \$\begingroup\$ Your first point seems to have missed that he's using the variable years in the output. So your suggestions don't seem to make sense there. \$\endgroup\$ – Winston Ewert Jul 6 '13 at 23:09
  • \$\begingroup\$ You're right, sorry about that. Edited. \$\endgroup\$ – user27011 Jul 6 '13 at 23:18
  • \$\begingroup\$ Hi Paul, I actually do plan to add in a type check as I'm a fan of error checking I just haven't done that yet. I just like to write efficient code and make checks along the way. So regarding the quotes, I don't really have a position. Even though I say that, if I'm right, to quote within there I would have to use double and single quotes so I am probably more likely to use double and then use single as necessary. I'm REALLY new to all of this so I appreciate your input. \$\endgroup\$ – Spurnout Jul 7 '13 at 2:57

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.