4
\$\begingroup\$

Here is my program that calculates tuition cost for the next five years. I have had some review on it so far, but I am seeking more. Please keep in mind I am new to programming, so I may require a few extra steps to understand something. Thanks in advance!

Version 1.0 is here

RESIDENCY_COST = {
                    "I": 10000,
                    "O": 24000,
                    "G": 40000,
                 }


cost_of_tuition = None
while True:
    residency = raw_input('Please input your type of residency, I for in-state, O for out-of-state, and G for graduate: ')
    try:
        cost_of_tuition = RESIDENCY_COST[residency]
        break
    except KeyError:
        print ('Please enter I, G or O ONLY.')


years = []
tuition_increase = []
academic_year = []
academic_year_inc = []

for _ in range(5):
    intMath = cost_of_tuition * 0.03
    tuition_increase.append(intMath)
    fnlMath = intMath + cost_of_tuition
    years.append(fnlMath)
    cost_of_tuition = fnlMath
    academic_year.append("${:,.2f}".format(fnlMath))
    academic_year_inc.append("${:,.2f}".format(intMath))
    total_tuition_increaseSum = sum(tuition_increase)
    total_tuition_increase = "${:,.2f}".format(total_tuition_increaseSum)



print('UNDERGRADUATE TUITION FOR THE NEXT FIVE YEARS ')
print('ACADEMIC YEAR          TUITION       INCREASE ')
print('-------------     ------------       -------- ')

for i, year in enumerate(range(16, 21)):
    print('{}-{}             {}        {}'.format(year + 2000, year + 1,
                                                  academic_year[i],
                                                  academic_year_inc[i]))

print('TOTAL TUITION INCREASE              ' + total_tuition_increase)
\$\endgroup\$
3
  • 2
    \$\begingroup\$ You still aren't compliant with the style guide (and mixing styles, such as total_tuition_increaseSum, is very bad form). \$\endgroup\$
    – jonrsharpe
    Commented Sep 30, 2015 at 13:19
  • \$\begingroup\$ @jonrsharpe Must have missed that one, thanks for pointing that out! I have also changed intMath and so on until I think of better variable names for that block.. \$\endgroup\$
    – Aaron
    Commented Sep 30, 2015 at 13:46
  • \$\begingroup\$ @holroy Sorry, I'm very new to the stackexchange. Is there a way to accept multiple answers? \$\endgroup\$
    – Aaron
    Commented Sep 30, 2015 at 16:25

2 Answers 2

2
\$\begingroup\$

You don't use total_tuition_increaseSum for anything other than that one print, so you may as well call sum on it in the print line:

total_tuition_increase = "${:,.2f}".format(sum(total_tuition_increase))

I would also recommend you look more at splitting the code into functions. Functions usually make code easier to read, use and change. For example, you might put your input management into one function so it's a small self contained chunk. All it needs to do that's different is return the resulting tuition cost instead of setting it directly. This is how it'd work:

def get_cost():
    while True:
        residency = raw_input('Please input your type of residency, I for in-state, O for out-of-state, and G for graduate: ')
        try:
            return RESIDENCY_COST[residency.upper()]
            break
        except KeyError:
            print ('Please enter I, G or O ONLY.')

And you could call it like this:

cost_of_tuition = get_cost()

Similarly, you could then wrap your printing into a print_tuition_table function. This one would need to take parameters, your two lists and total_tuition_increase.

def print_tuition_table(academic_year, academic_year_inc, total_tuition_increase):
    print('UNDERGRADUATE TUITION FOR THE NEXT FIVE YEARS ')
    print('ACADEMIC YEAR          TUITION       INCREASE ')
    print('-------------     ------------       -------- ')

    for i, year in enumerate(range(16, 21)):
        print('{}-{}             {}        {}'.format(year + 2000, year + 1,
                                                      academic_year[i],
                                                      academic_year_inc[i]))

    print('TOTAL TUITION INCREASE              {}'.format(total_tuition_increase))

Of course, you could change this to be more re-usable now if you didn't hard code it as being five years. Instead, you could check the length of the academic_year list and then just change the way the loop works slightly:

def print_tuition_table(academic_year, academic_year_inc, total_tuition_increase):
    length = len(academic_year)
    print('UNDERGRADUATE TUITION FOR THE NEXT {} YEARS '.format(length))
    print('ACADEMIC YEAR          TUITION       INCREASE ')
    print('-------------     ------------       -------- ')

    for i, year in enumerate(range(16, 16 + length)):
        print('{}-{}             {}        {}'.format(year + 2000, year + 1,
                                                      academic_year[i],
                                                      academic_year_inc[i]))

    print('TOTAL TUITION INCREASE              ' + total_tuition_increase)

Then you could do the same with your calculation loop. It would need to take in cost_of_tuition and return multiple values, but return multiple values is easy in Python. First, you wrap it in a function like this:

def calculate_fees(cost_of_tuition):
    for _ in range(5):
        intMath = cost_of_tuition * 0.03
        tuition_increase.append(intMath)
        fnlMath = intMath + cost_of_tuition
        years.append(fnlMath)
        cost_of_tuition = fnlMath
        academic_year.append("${:,.2f}".format(fnlMath))
        academic_year_inc.append("${:,.2f}".format(intMath))
        total_tuition_increaseSum = sum(tuition_increase)
        total_tuition_increase = "${:,.2f}".format(total_tuition_increaseSum)

    return academic_year, academic_year_inc, cost_of_tuition

Then you can set those values like this:

academic_year, academic_year_inc, cost_of_tuition = calculate_fees(cost_of_tuition)

Also, you could easily expand this function to take a years parameter so that it doesn't have to be stuck with 5 years.

def calculate_fees(cost_of_tuition, years):
    for _ in range(years):

And since our printing function checks the length of your list, now you just need to change one number to be able to adjust the whole output.

With all those functions defined, here's how you could call them to run the script:

cost_of_tuition = get_cost()
academic_year, academic_year_inc, cost_of_tuition = calculate_fees(cost_of_tuition)
print_tuition_table(academic_year, academic_year_inc, cost_of_tuition)
\$\endgroup\$
5
  • \$\begingroup\$ Nothing but amazing feedback from you so far, thanks a whole lot for the guidance and depth of your feedback. I appreciate it! \$\endgroup\$
    – Aaron
    Commented Sep 30, 2015 at 15:23
  • \$\begingroup\$ The only error I'm getting is....:Traceback (most recent call last): File "xxxxxx", line 59, in <module> print_tuition_table(academic_year, academic_year_inc, cost_of_tuition) File xxxxx", line 55, in print_tuition_table print('TOTAL TUITION INCREASE ' + total_tuition_increase) TypeError: cannot concatenate 'str' and 'float' objects >>> \$\endgroup\$
    – Aaron
    Commented Sep 30, 2015 at 15:34
  • 1
    \$\begingroup\$ Ah,l should've made that a format call. It's fixed now if you take another look at that line. \$\endgroup\$ Commented Sep 30, 2015 at 15:36
  • \$\begingroup\$ Do you have time for one of those chats again? I'm unsure of how to initiate one.. \$\endgroup\$
    – Aaron
    Commented Sep 30, 2015 at 15:39
  • \$\begingroup\$ @Xarotic Sure, I'm in here: chat.stackexchange.com/rooms/29711/… \$\endgroup\$ Commented Sep 30, 2015 at 15:40
1
\$\begingroup\$

Code review

Some code issues:

  • Use clear an intuitive variable names – Clearer variable names help understand the code better, i.e. intMath should most likely be interest, fnlMath is possibly new_tuition (or actually it is the new cost_of_tuition)
  • Remove unused variables – You are not using years any more, so remove it
  • Separate business logic from presentations – In general, it is better to store the actual values, and not the formatted version of the value. That is store the cost_of_tuition and interest instead of the formatted version. Format then when you print the values
  • Delay sums until they are needed – In your code you do a sum over the increase, for each year, but you only use that number for a total increase. Delay this sum until you need it
  • Avoid magic numbers – You are currently using two magic numbers, 5 and 0.03. Either declare them as constants, or let them be parameters into your functions
  • Use functions where appropriate – I don't know if you've covered this already in your course, but your code could/should be coded in three separate functions: select_initial_cost(), calcuclate_costs(), print_costs(). And for ease of reuse, it can be wise to introduce constants as parameters to function, or in the case of select_inital_cost() to allow for specifying it directly (with default to input it)
  • Adjust columns according to width – Formatting of columns based upon fixed width assumption are never good. You're better of introducing the width of the column somehow. This do however easily get ugly when you want to do both formatting of floats and inserting the $ in front of your currency. In the example code given below, I've only adjusted for the width and left the currency sign to the far left of the column

With all these corrections the code now looks like the following:

from datetime import date

RESIDENCY_COST = {
    "I": 100,
    "O": 2400,
    "G": 40000,
}

INTEREST_RATE = 0.03
NUMBER_OF_YEARS = 5
START_YEAR = date.today().year

def select_initial_cost(residency=None):
    """Initial cost is selected based on residency type"""
    if residency in RESIDENCY_COST:
        return RESIDENCY_COST[residency]

    while True:
        residency = raw_input('Please input your type of residency, I for in-state, O for out-of-state, and G for graduate: ')
        try:
            return RESIDENCY_COST[residency]
        except KeyError:
            print ('Please enter I, G or O ONLY.')

def calculate_costs(initial_cost, number_of_years=NUMBER_OF_YEARS, interest_rate=INTEREST_RATE):
    """Calculate a table of cost for a range of years, based on 
       initial_cost, number of years and interest rate.

       The list returned consists of the year, accumulated cost, and this year increase
       (or interest) for the following number of years
    """
    academic_year = []
    cost_of_tuition = initial_cost

    for year in range(number_of_years):
        interest = cost_of_tuition * interest_rate
        cost_of_tuition += interest

        academic_year.append( (START_YEAR + year, cost_of_tuition, interest) )

    return academic_year


def print_cost(academic_year):
    """Print all values related to academic years"""
    print('\nUNDERGRADUATE TUITION FOR THE NEXT {} YEARS\n'.format(len(academic_year)))

    print('Academic year     Tuition        Increase')
    print('---------------   ------------   ------------')
    sum_interest = 0
    for (year, tuition, interest) in academic_year:
        sum_interest += interest
        print('{:<15}   ${:>11,.2f}   ${:>11,.2f}'.format('{}-{}'.format(year, year + 1),
                                                  tuition,
                                                  interest))

    print('{:>30}   ${:11,.2f} '.format('Total tuition increase:', sum_interest))
    #sum_interest2 = sum(interest for (_, _, interest) in academic_year)


# Your initial code
initial_cost = select_initial_cost()
academic_year = calculate_costs(initial_cost)
print_cost(academic_year)

Due to the addition of the magic numbers as parameters you can now also do stuff like:

for residency in ('I', 'O', 'G'):
    initial_cost = select_initial_cost(residency)

    academic_year = calculate_costs(initial_cost, number_of_years=10, interest_rate = 0.05)
    print_cost(academic_year)
\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot for your feedback! Especially about the variable names.. I wasn't really sure what I should be calling those. I am very new to the language and seem to get ahead of myself sometimes. \$\endgroup\$
    – Aaron
    Commented Sep 30, 2015 at 16:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.