0
\$\begingroup\$

The code is about displaying membership information of all clients from a database. This is for homework and I'm just curious if more can be done to improve this. Feel free to alter it, but please include some reasons for your alterations.

import sys
import re

def menu():
    print("---------------Tracking Jobs and Revenue---------------")
    print("     Option A: Search for an estimate                  ")
    print("     Option B: Display outstanding payments            ")
    print("     Option C: Display total revenue                   ")
    print("                                                       ")
    print("     Enter Q to quit                                   ")
    print("-------------------------------------------------------\n")

def choiceOptions():
    global DoAnother
    DoAnother = True
    while DoAnother == True:
        print("")
        choice = input("Please enter an option of your choice(A/B/C/Q):\n")
        if choice == "A":
            optionA()
            DoAnother = False
        elif choice == "B":
            optionB()
            DoAnother = False
        elif choice == "C":
            optionC()
            DoAnother = False
        elif selection == "Q":
            endprog()
            DoAnother = False
        else:
            print("")
            print("Please enter a valid letter from the list shown")
            return choiceOptions()

def optionA():
    global DoAnother
    while DoAnother == True:
        print("")
        estimate = input("Please enter the estimate number:")
        if len(member) !=5:
            print("That isn't a valid estimate number. Please try again")
            DoAnother = True
        else:
            global estimatefile
            estimatefile = open("clientRecords","r+")
            searchEst = True
            for line in estimatefile.readlines():
                listed = line.split(",")
                if member in lister:
                    print("")
                    print("Estimate Number: {0}".format(listed[0]))
                    print("CustomerID: {0}".format(listed[1]))
                    print("Final Total: {0}".format(listed[2]))
                    print("Amount Paid: {0}".format(listed[3]))
                    print("Amount Outstanding: {0}".format(listed[4]))
                    print("Status: {0}".format(listed[5]))
                    searchEst = False
                    nextchoice()
                if searchEst == True:
                    optionA()

def optionB():
    print("")
    print("--------------------------------------------------------------")
    print("Estimate Number|CustomerID|Etimate Amount|Estimate Date|Status")
    global estimatefile
    estimatefile = open("clientRecords","r+")
    amountOut = 0
    for line in estimatefile.readlines():
        listed = line.split(",")

        if listed[5] == "A" and listed[3].strip('\n')<listed[2]:
            difference = (int(listed[5]-int(listed[2]))
            amountOut = (amountOut + difference)
            print("{0}     {1}     £{2}     £{3}     £{4}     {5}".format(listed[0], listed[1], listed[2], listed[3], listed[4], listed[5].strip('\n'), difference))
    print("")
    print("                                 Total Outstanding: £{0}".format(amountOutstanding))
    nextchoice()

def optionC():
    global estimatefile
    estimatefile = open("clientRecords","r+")
    global amountOwed
    amountOwed = 0
    for line in estimatefile.readlines():
        listed = line.split(",")
        if listed[5] == "A" and listed[3].strip('\n') == listed[5]:
            amountOwed = amountOwed + int(listed[3])
    print("")
    print("Total Revenue")
    print("")
    print("The company's total revenue so far is £{0}".format{amountOwed))
    nextchoice()

def nextchoice():
    global DoAnother
    while DoAnother == True:
        print("")
        option = input("Would you like to choose another option(Y/N)?")
        if option == "Y":
            print("")
            DoAnother = False
            main()
        elif option == "N":
            print("")
            DoAnother = False
            endprog()
            break
        else:
            print("")
            print("Please enter a valid letter")
            DoAnother = True

def endprog():
    print("\n------------------------")
    print("        Thank you 
    print("------------------------")

def main():
    menu()
    selectOption()

main()
\$\endgroup\$
2
  • \$\begingroup\$ Can you please include a hint to the purpose of your program in the question title? \$\endgroup\$ Commented Apr 24, 2016 at 21:34
  • 5
    \$\begingroup\$ While it is acceptable to ask follow-up questions on Code Review, we do have some common-sense etiquette rules. 1) In the old question, add a comment linking to the new question. 2) In the new question, mention your previous question and what advice you have incorporated from previously received answers. As it stands, this is nearly a duplicate of your previous question. \$\endgroup\$ Commented Apr 25, 2016 at 5:45

1 Answer 1

3
\$\begingroup\$

Oh man, oh man. Took me way too long fixing ridiculous amounts of unresolved variable names, random indention issues, misuse of basic Python operators, misspelled variable names, and a load of other small issues that should have been caught before posting to code review. What you posted was hard to get up and running, but I guess that's why we're here to help. Just make sure you comb through your code a little better before posting!

In any case, I commented where most of the issues were. I'm a Python 2.7 guy, so minor changes have been made to accommodate for my version of Python, e.g. stuff like 'input' to 'raw_input'.

There are still issues in function OptionA that I just could not fix.

You don't have to constantly re-declare variables as global, nor do you have to make up booleans for basic while loops. Just use 'while True' in combination with 'break' and 'continue' for the respective purposes.

I'm sure I missed a thing here or there, because I was looking at so many issues.

import sys
# Removed 'import re'

def choiceOptions():
    # Deleted 'DoAnother' becuase it is completely useless. Just use 'while True:'
    while True:
        print("---------------Tracking Jobs and Revenue---------------")
        print("     Enter A: Search for an estimate                  ")
        print("     Enter B: Display outstanding payments            ")
        print("     Enter C: Display total revenue                   ")
        print("                                                       ")
        print("     Enter Q to quit                                   ")
        print("-------------------------------------------------------")
        print("")
        # Added these print statements because it made no sense to have them in a seperate function.
        choice = raw_input("Please enter an option of your choice(A/B/C/Q):\n").lower()
        # '.lower()' was added so that 'choice' will only represent lower case characters.
        # This is done so that confusion between upper and lower case raw_input entries
        # cannot be confused in the following if statements.
        # If this is not done, the user must capitalize their input for no reason.
        # Alternatively, you could use '.upper()', if you really feel you need to.
        if choice == "a":
            # Replaced all 'DoAnother = False' to break statements, so that this loop doesn't endlessly continue.
            optionA()
            break
        elif choice == "b":
            optionB()
            break
        elif choice == "c":
            optionC()
            break
        elif choice == "q":
            # Changed 'selection == 'q':' to 'choice == 'q'':
            endprog()
            break
        else:
            print("")
            print("Please enter a valid letter from the list shown")
            continue
            # Just use continue to continue the loop.

def optionA():
    # 'while True'!
    while True:
        print("")
        estimate = input("Please enter the estimate number:")
        if estimate <= 5:
            # This was judging the length of a 'member' variable that doesn't exist.
            # Cannot use len() w/ int values.
            # Big estimate numbers, or weird int limitation estimate?
            print("That isn't a valid estimate number. Please try again")
            continue
        else:
            global estimatefile
            estimatefile = open("clientRecords","r+")
            searchEst = True
            for line in estimatefile.readlines():
                listed = line.split(",")
                """if member in lister:
                    print("")
                    print("Estimate Number: {0}".format(listed[0]))
                    print("CustomerID: {0}".format(listed[1]))
                    print("Final Total: {0}".format(listed[2]))
                    print("Amount Paid: {0}".format(listed[3]))
                    print("Amount Outstanding: {0}".format(listed[4]))
                    print("Status: {0}".format(listed[5]))
                    searchEst = False
                    nextchoice()"""
            # 'member' was only found once, but in reference to the estimate amount, and 'lister' isn't mentioned
            # in the file at all.  You cannot have unresolved references to variables that don't exist.
                if searchEst == True:
                    continue

def optionB():
    print("")
    print("--------------------------------------------------------------")
    print("Estimate Number|CustomerID|Etimate Amount|Estimate Date|Status")
    global estimatefile
    estimatefile = open("clientRecords","r+")
    amountOut = 0
    for line in estimatefile.readlines():
        listed = line.split(",")

        if listed[5] == "A" and listed[3].strip('\n')<listed[2]:
            difference = (int(listed[5]-int(listed[2])))
            amountOut += difference
            # Use the operator '+=' or '-=' when writing a variable that includes an addition/subtraction of itself.
            # i.e. 'x = x + y' should just be 'x += y'; 'a = a - b' becomes 'a -= b'
            print("{0}     {1}     ${2}     ${3}     ${4}     {5}".format(listed[0], listed[1], listed[2], listed[3], listed[4], listed[5].strip('\n'), difference))
            print''
            """print("                                 Total Outstanding: ${0}".format(amountOutstanding))"""
            # 'amountOutstanding' is another unresolved reference to a fictional variable.
            nextchoice()

def optionC():
    global estimatefile
    estimatefile = open("clientRecords","r+")
    global amountOwed
    amountOwed = 0
    for line in estimatefile.readlines():
        listed = line.split(",")
        if listed[5] == "A" and listed[3].strip('\n') == listed[5]:
            amountOwed = amountOwed + int(listed[3])
    print("")
    print("Total Revenue")
    print("")
    print("The company's total revenue so far is ${0}".format(amountOwed))
    nextchoice()

def nextchoice():
    # 'while True:'!
    while True:
        print("")
        option = input("Would you like to choose another option(Y/N)?").lower()
        # Added '.lower()' again.
        if option == "y":
            print("")
            main()
            break
            # Added Break statements.
        elif option == "n":
            print("")
            endprog()
            break
        else:
            print("")
            print("Please enter a valid letter")
            continue

def endprog():
    print("\n------------------------")
    print("        Thank you         ")
    print("------------------------")
    # Added a 'sys.exit()' so that the program may actually exit.
    sys.exit()

def main():
    choiceOptions()
    # Changed 'selectOption()' to 'choiceOptions() and got rid of 'menu()'.

main()
\$\endgroup\$
2
  • 4
    \$\begingroup\$ You aren't meant to review your own code. If his code isn't working, don't change it and review that; wait until it is working. One more thing, Welcome to Code Review! \$\endgroup\$
    – zondo
    Commented Apr 24, 2016 at 22:53
  • \$\begingroup\$ Hey thanks haha. I was just trying to help. I'll keep that in mind! \$\endgroup\$ Commented Apr 24, 2016 at 23:28

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.