12
\$\begingroup\$

I've decided to start working on a budget application for the purposes of enhancing my Python skills. In its current form this program takes given expenses and income and tells the user whether they have a surplus, a deficit, or if they are breaking even. I will add more functionality to this program as I move along in the development process.

import os
import sys

class Application():
    def __init__(self):
        self.income = 0
        self.expenses = 0
        self.expense_list = []
        self.expense_name = []
        self.income_name = []
        self.income_list = []
        self.prompt_income()

    def income_ask(self):
        add_income = input('Add income? [y/n]: ')
        return add_income

    def income_sum(self):
        self.income = sum(self.income_list)

    def expense_ask(self):
        add_expense = input('Add expense? [y/n]: ')
        return add_expense

    def expense_sum(self):
        self.expenses = sum(self.expense_list)

    def income_check(self):
        if not self.income_list:
            print('Please enter atleast one source of income. ')
            self.prompt_income()
        else:
            return

    def expense_check(self):
        if not self.expense_list:
            print('Please enter atleast one expense. ')
            self.prompt_expense()
        else:
            return

    def prompt_income(self):
        x = False
        while not x:
            result = self.income_ask()
            if result == 'y':
                income_input = int(input('Enter source of income. [Numbers Only]: '))
                self.income_list.append(income_input)
                income_name = input('Enter income name. [Name Only]: ')
                self.income_name.append(income_name)
            else:
                self.income_check()
                x = True
        self.income_sum()
        name = [name for name in self.income_name]
        income = [income for income in self.income_list]
        incomedict = dict(zip(name, income))
        for k in incomedict:
            print(k + ': ', '$' + str(incomedict[k]))
        print('Total user income: ', '$' + str(self.income))
        self.prompt_expense()

    def prompt_expense(self):
        x = False
        while not x:
            result = self.expense_ask()
            if result == 'y':
                expense_input = int(input('Enter expense amount. [Numbers Only]: '))
                self.expense_list.append(expense_input)
                expense_name = input('Enter expense name. [Name Only]: ')
                self.expense_name.append(expense_name)
            else:
                self.expense_check()
                x = True
        self.expense_sum()
        name = [name for name in self.expense_name]
        expense = [income for income in self.expense_list]
        expensedict = dict(zip(name, expense))
        for k in expensedict:
            print(k + ': ', '$' + str(expensedict[k]))
        print('Total user expenses: ', '$' + str(self.expenses))
        self.uservalue()

    def uservalue(self):
        valoutput = self.income - self.expenses
        if valoutput < 0:
            print('You are in the negative, you have a deficit of ' + '$' + str(valoutput))
        if valoutput == 0:
            print('You have broken even, you are spending exactly as much as you make.')
        if valoutput > 0:
            print('You are in the positive, you have a surplus of ' + '$' + str(valoutput))
        another = input('Would you like to run another analysis? [y/n]: ')
        if another == 'y':
            self.reset_program()
        else:
            self.close_program()

    def reset_program(self):
        self.income = 0
        self.expenses = 0
        del self.expense_list[0:]
        del self.expense_name[0:]
        del self.income_name[0:]
        del self.income_list[0:]
        self.prompt_income()

    def close_program(self):
        print('Exiting Program.')
        sys.exit(0)

if __name__ == '__main__':
Application()
\$\endgroup\$

4 Answers 4

11
\$\begingroup\$

Here are the few things I've noticed after a first glance on the program:

  • import os is not used, you can remove it
  • () after the class Application can be omitted
  • keep two blank lines between the imports and the class definition, after the class code and the if __name__ == '__main__' line
  • variable naming - there are some poor variable names like x or k - consider renaming them to something more descriptive
  • the following code block can be removed (unless you have it there as a stub for future development):

    else:
        return
    
  • the class itself feels overloaded - this may be because it does two different not-directly related things at the same time - prompting the user and making budget calculations - since you are going to continue to develop the class - make sure Application is not becoming a God Object

  • the class and its methods would benefit from having detailed documentation strings

As a side note, take a look at the The Architecture of Open Source Applications project - it is really pure informational gold in terms of designing, thinking and development of your software project.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks, all of these are informative. The only item you've listed that I don't understand is the second point, what is the reasoning behind omitting the () afterclass Application? EDIT: Thanks for the link as well! \$\endgroup\$ Aug 21, 2017 at 0:03
  • 1
    \$\begingroup\$ @NoOrangeJuice right, the opening and closing parenthesis can be remove leaving only class Application:. Sorry for poor wording there. Thanks. \$\endgroup\$
    – alecxe
    Aug 21, 2017 at 0:04
  • \$\begingroup\$ I see ok, thanks again for this link. I'm reading it now and its full of good information. \$\endgroup\$ Aug 21, 2017 at 0:07
5
\$\begingroup\$

DRY and Data Structures

Income and expenses are the same thing, just with different signs. You can cut about half your code by more fully leveraging this.

Currently, each of the two have a list of values, a list of labels, a sum variable, a prompt function, a "check", and a sum function. A more flexible data design might look like this:

Entry = {
    value: <int>,
    label: <str>
}

entries = [Entry]

so that the value and label of each entry is stored together, which is a lot cleaner than the dict(zip(... business. An expense would just be an entry with a negative value.

You can still prompt for input in two phases, if you want, but just store them both in the same format internally.

You can then consolidate functions. Skipping a bit of error handling, your class becomes something more like:

class Application:

    def __init__(self):
        self.entries = []
        self.main()

    def prompt(self, type):
        while True:
            more = input('Enter %s? [y/n] ' % type).lower()
            if more == 'n':
                break

            value = int(input('Enter %s amount (numbers only): ' % type))
            label = input('Enter %s name' % type)

            self.entries.append({
                'value': value if type == 'income' else -value,
                'label': label
            })

    def main(self):
        self.prompt('income')
        self.prompt('expense')

        # This is easy now, since the sign is attached to the data
        balance = sum(entry['value'] for entry in entries)
        self.print_balance_message(balance)

Simplify

  • Do you need to print the list of income / expenses after the user entered it? Presumably the entire history is still on their screen?
  • Why not allow the user to enter both types at once? I'd be a little annoyed if I was entering my incomes, then expenses, then wait -- I forgot an income -- do I have to start over?
\$\endgroup\$
1
  • \$\begingroup\$ You make a good point for simplification. As I'm looking at it again, days away from when I initially wrote it, I can see that it is indeed very noisy and could be trimmed down. Thanks \$\endgroup\$ Aug 21, 2017 at 2:11
5
\$\begingroup\$
  • The while not x: loop can be replaced with a while True loop and you can break out of it using break statement:

    while True:
        result = self.income_ask()
        if result == 'y':
            ...
        else:
            ...
            break
    
  • Try not to include the name of the data-structure(list, dict etc) in your variable names. self.income_list could be replaced with self.incomes and self.income_name should be self.income_names.

  • These two list-comprehensions are unnecessary name = [name for name in self.income_name] and income = [income for income in self.income_list]. incomedict could directly use dict(zip(self.income_name, self.income_list)). Plus again the variable names name and income should be names and incomes.

  • You can iterate over (key, value) pairs of a dict using .items(): for name, income in incomedict.items():.

  • Use string formatting to make your code more-readable, for example: print('You are in the negative, you have a deficit of ${amount}.format(amount=valoutput)). If you're on Python 3.6+ then look into f-strings as well.

  • You should only be using incomedict and expensedict instead of maintaining four names and expense/income lists. Apart from saving space another advantage is that a user can repeat a same name again and you will be able to handle it easily by adding it to existing name.

  • There's too much state modification happening in your methods. Try to do that as minimum as possible because it makes the code very hard to debug and understand. For example it's not clear what self.expense_sum() does, but it updates the self.expenses. It should ideally be a property returning the sum and in rare cases if you do need to update the state then make sure the method is named appropriately i.e. self.update_expenses_sum.

\$\endgroup\$
2
\$\begingroup\$

In addition to the other remarks:

reset_program does exactly the same as __init__, so you could either call that directly:

    if another == 'y':
        self.__init__()

or by creating a new instance altogether:

    if another == 'y':
        Application()

The latter is nice because you are certain you won't have any attributes from the last run leftover, messing with your methods, the former is nice because you don't have a multitude of instances (I am not sure what happens with the first application when you create another one, since it isn't stored anywhere).

Not exactly knowing which way was the cleanest to do this, I asked a question on SO about resetting classes.

It seems that calling __init__() or any __something__-methods directly is bad form. We could however move the definition of the class attributes to reset_program and do the following:

def __init__():
    self.reset_program()

The second option above only works because you run the entire functionality of the class from within the __init__() method (which may or may not be bad form). When the new instance is created, the back and forth prompt is executed right away, so the moment it would go out of scope and get deleted (because it isn't assigned anywhere), is only after you are done with your input and output (and possibly been given anyother option to reset).

\$\endgroup\$
5
  • 1
    \$\begingroup\$ Since I am not all that experienced with Python, I would personally be interested which of the two methods would be preferred. Feel free to edit that into the answer. \$\endgroup\$
    – JAD
    Aug 21, 2017 at 10:30
  • \$\begingroup\$ I'm not sure which would be more preferable, I'm curious what others have to say about this well. \$\endgroup\$ Aug 21, 2017 at 16:07
  • 1
    \$\begingroup\$ @NoOrangeJuice I asked a question about it \$\endgroup\$
    – JAD
    Aug 21, 2017 at 16:39
  • 1
    \$\begingroup\$ @NoOrangeJuice see edit \$\endgroup\$
    – JAD
    Aug 22, 2017 at 6:30
  • \$\begingroup\$ This looks like the cleanest solution. I enjoyed the question you asked, it was very interesting to see how many different methods of closing and/or resetting a program exist and how some methods may be more suitable than others depending on the situation. \$\endgroup\$ Aug 22, 2017 at 16:30

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.