Displaying member information from a CSV file

Question

The code is about displaying membership information of all clients from database. I was just wondering what I could do to improve/change it.

def optionA():
    global DoAnother
    while DoAnother == True:
        print("")
        member = input("Please enter the member number: ")
        if len(member) !=4:
            print ("")
            print("The member number is not valid")
            DoAnother = True
        else:
            global memberfile
            memberfile = open("members.txt","r+")
            searchMem = True
            for line in memberfile.readlines():
                listed = line.split(",")
                if member in listed:
                    print("")
                    print("Team Code: {0}".format(listed[0]))
                    print("Member number: {0}".format(listed[2]))
                    print("Date of joining: {0}".format(listed[1]))
                    print("Membership type: {0}".format(listed[3]))
                    print("Amount paid: {0}".format(listed[5]))
                    searchMem = False
                    nextchoice()
            if searchMem == True:
                optionA()

def optionB():
    print("")
    print("Team Code  Member No           Fee      Amount Paid      Amount Outstanding")
    print("----------------------------------------------------------------------------------------")
    global memberfile
    memberfile=open("members.txt","r+")
    amountOut = 0
    for line in memberfile.readlines():
        listed = line.split(",")

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

Answer 1

There are a number of issues with this code; the first two items below are seriously bad habits.

• Inappropriate use of functions. Functions are not goto labels. Having optionA call itself or nextchoice() makes no sense. This must be corrected — it is a fundamental misunderstanding of how to write programs.
• Use of global variables. If you design your functions properly, there should be no need for any global variables.
• Lack of abstraction. There is nothing in common between these two functions, other than the fact that they both read from members.txt. It would be a good idea to have a function to read, interpret, search the file. Furthermore, since the data are in a csv format, you should use the Python csv module.

• Using string comparison for numbers. This is a bug:

  listed[5].strip('\n')<listed[4]
  

  … because you are comparing numbers ASCIIbetically.

• Lack of documentation. optionA and optionB are both mysterious names for functions. It's not immediately clear what the members.txt file looks like, or why optionB ignores rows whose membership type column is not F.
• File descriptor leak. If you call open() without a corresponding call to close(), you might leak file descriptors. In Python, you almost always want to call open() using a with block.

• Member search by all attributes. You ask the user to enter a member number. However, when you do

  listed = line.split(",")
  if member in listed:
      …
  

  … you are actually searching all fields, not just the third column.

• Python style guide. PEP 8, the official Python style guide, says that variable names should be lower_case_with_underscores unless you have a good reason to deviate from the norm.

Suggested solution

import csv
from decimal import Decimal

MEMBERS_TXT_FIELDS = [
    ('Team Code', str),
    ('Date of joining', str),
    ('Member number', str),
    ('Membership type', str),
    ('Fee', Decimal),
    ('Amount paid', Decimal),
]

def members(member_number=None):
    with open('members.txt') as member_file:
        csv_file = csv.reader(member_file)
        for row in csv_file:
            member = {
                field_name: conv(val)
                for (field_name, conv), val in zip(MEMBERS_TXT_FIELDS, row)
            }
            if member_number is None or member_number == member['Member number']:
                yield member

def show_single_member(member_number):
    for member in members(member_number):
        print("""
Team Code: {Team Code}
Member number: {Member number}
Date of joining: {Date of joining}
Membership type: {Membership type}
Amount paid: {Amount paid}""".format(**member))
        return True
    return False

def show_owing_members():
    def value_fmt(field_def, member):
        field_name, width = field_def
        value = member[field_name]
        if isinstance(value, Decimal):
            return "£{0}".format(value.quantize(Decimal('0.01'))).rjust(width - 1) + ' '
        else:
            return str(member[field_name]).ljust(width)

    TABLE_FIELDS = [
        # Field name, width
        ('Team Code', 11),
        ('Member number', 20),
        ('Fee', 9),
        ('Amount paid', 17),
        ('Amount outstanding', 31),
    ]

    # Print headings
    print(''.join(field.ljust(width) for field, width in TABLE_FIELDS))
    print(''.join('-' * width for field, width in TABLE_FIELDS))

    total_outstanding = Decimal('0.00')
    for member in members():
        if member['Membership type'] == 'F' and member['Amount paid'] < member['Fee']:
            member['Amount outstanding'] = member['Fee'] - member['Amount paid']
            print(''.join(value_fmt(field_def, member) for field_def in TABLE_FIELDS))
            total_outstanding += member['Amount outstanding']
    print()
    w = sum(width for field, width in TABLE_FIELDS if field != 'Amount outstanding')
    print("Total outstanding: ".rjust(w) + "£{0}".format(total_outstanding))


def menu():
    while True:
        print("A: Member lookup")
        print("B: Outstanding fees")
        print("Q: Quit")
        choice = input("Choice: ").upper()
        if choice == 'A':
            ok = show_single_member(input("Please enter the member number: "))
            if not ok:
                print("No such member")
        elif choice == 'B':
            show_owing_members()
        elif choice == 'Q':
            break
        print()

Answer 2

I would implement something more functional, that way if you go on to create a larger business program you could debug, and section your needs better.

#seperating the formatting/print function from the lookup would be alot cleaner
#and allow for long-term changes without having to mess with the parts that work

def getMemberInfo():

    #you also don't need loop values, rather keep testing bad input
    #as the looping mechanism
    member = input("Please enter the member number: ")
    while len(member) != 4:
        member = input("\nPlease enter a valid member number: ")

    #cleaner way to open files
    with open("members.txt", "r+") as memberFile:
        for line in memberFile:
            listed = line.split(",")
            if member in listed:

                #append the difference value, to avoid anything other
                #than formatting in the second function
                listed.append(int(listed[5]) - int(listed[4]))
                return listed

            #else:  read next line
        return(None) # member wasn't found in file       

def printMemberInfo( listed ):    
    print("")
    print("")
    print("Team Code: {0}".format(listed[0]))
    print("Member number: {0}".format(listed[2]))
    print("Date of joining: {0}".format(listed[1]))
    print("Membership type: {0}".format(listed[3]))
    print("Amount paid: {0}".format(listed[4]))

    #you can play around with the formatting
    #exceeding col: 80 was irking me

    print("{0} {1} £{2} £{3} £{4}".format( \
           listed[0], listed[2], listed[4], listed[5].strip('\n'), listed[6]))

    print("")
    print(" Total Outstanding: £{0}".format(listed[6]))

With this code, you could have something that determines what the user wants to do, maybe getMemberInfo(), updateMemberInfo(), deleteMember(), and then can call those respective functions inside a main(): eventloop!