Programming Forums - Critique my exercise

Programming Forums (http://www.programmingforums.org/forumindex.php)

- Python (http://www.programmingforums.org/forum43.html)

- - Critique my exercise (http://www.programmingforums.org/showthread.php?t=10295)

Critique my exercise

Let me start by saying that this was just an exercise that I made up for myself while learning python now that I'm a couple of weeks in. It began originally as just a means to play with dictionary variables, but I ended up adding some file i/o and error handling.

The program works exactly as it should. I've fixed all the bugs that I have found thus far... I'm just curious to see if the way I wrote this is the best (or good enough). Should I have done something differently? Is there unnecessary code?

#!/usr/bin/python



###############################

#

# This program will track things that you have loaned to your friends

#

#



import string



# Function definitions



# Draw menu



def menu():

    print

    print "1. Add person"

    print "2. Add item to person"

    print "3. Remove item from person"

    print "4. Remove person"

    print "5. View all loaned items"

    print "9. Save data and exit"

    print



# End function definitions



dbase = {}



try:                        # Open the data file and import it

    inFile = open("data.txt", "r")

    while 1:

        inLine = inFile.readline()

        if len(inLine) == 0:

            break

        temp = inLine.split(',')

        dbase[temp[-2]] = temp[:-2]

        

    inFile.close()

except IOError:             # If no file found, we'll make a new one

        print "No saved data found.  Creating new file..."

    

choice = "0"

while choice != "9":        # main menu loop

    menu()

    choice = raw_input("Selection:")

    if choice == "1":       # add person

        name = raw_input("Enter name:")

        dbase[name] = []

        



    elif choice == "2":     # add item

        name = raw_input("Person's name:")

        if dbase.has_key(name):

            item = raw_input("Item:")

            dbase[name].append(item)

        else:

             print name, "was not found."





    elif choice == "3":     # remove item

        name = raw_input("Enter name from which to remove item:")

        if dbase.has_key(name):

            item = raw_input("Enter item to remove:")

            if item in dbase[name]:

                number = dbase[name].index(item)

                del dbase[name][number]

            else:

                print "Item not found."

        else:

            print "Name not found."





    elif choice == "4":     # remove person

        name = raw_input("Enter name to delete:")

        if dbase.has_key(name):

            if not len(dbase[name]) == 0:

                print name, "still has items on loan."

                certain = raw_input("Delete anyway? (y/N):")

                if certain == "y" or certain == "Y" or certain == "yes":

                    del dbase[name]

                else:

                    print ""

            else:

                del dbase[name]

        else:

             print name, "was not found."





    elif choice == "5":     # view all items

         for x in dbase.keys():

            print "%s:" % (x),

            for y in dbase[x]:

                if dbase[x].index(y) != len(dbase[x]) -1:

                    print "%s," % (y),

                    

                else:

                    print y

            print""



print ""



# Now we save the file

try:

    outFile = open("data.txt", "w")

    for x in dbase.keys():

        dbase[x].append(x)

        for y in dbase[x]:

            output = y

            output += ","

            outFile.write(output)

        outFile.write("\n")

    outFile.close()

    print "Data saved..." 



except NameError:

    print "No data saved..."

I don't know much about Python but I can understand the basics of what your code does. If your only a couple weeks into it, I'd say you did a great job long as you understand what it all does :) (not just cutting and pasting, but it doesn't seem that way). Good begining program ;) Nice commenting and what looks to be perfect indenting. Nice clean code, Great job dude :)

like booooze said that is a very nice clean looking code. i think that is great program you made just learning python.

Quote:

Originally Posted by somehollis

The program works exactly as it should. I've fixed all the bugs that I have found thus far... I'm just curious to see if the way I wrote this is the best (or good enough). Should I have done something differently? Is there unnecessary code?

There are a few things that could be done differently. Your file opening code for instance, could be more easily written thus:

try:

    file = open("data.txt")

    for line in file:

        if line == "": break

        values = line.split(',')

        dbase[values[-2]] = values[:-2]

finally:

    file.close()

The "finally" clause makes sure that even if there is an IOError, the file will be closed correctly. This code may need to be wrapped in a further try-except statement in order to ignore any IOErrors.

In fact, one could make it simpler still, and use the standard csv module:

import csv



try:

    file = open("data.txt", "rb")

    for row in csv.reader(file):

        if not row: break

        dbase[rows[-2]] = rows[:-2]

finally:

    file.close()

One could also use csv.writer to write to the file in a similar manner.

The second thing I'd do differently would be to use a dictionary instead of a lot of elif-statements:

def add_person():

    name = raw_input("Enter name:")

    dbase[name] = []



...



choices = {

    0 : add_person,

    1 : add_item,

    ...

}



while choice != 9:

    print menu

    try:

       choice = int(raw_input("Selection:"))

       choices[choice]()

    except ValueError:

       print "Invalid choice: must be a number"

    except KeyError:

       print "Invalid choice: number must be from 0 to 9"

The third thing I'd consider is whether the data can be saved in a different format. If data.txt is only read by this Python program, then it might be better to use the shelve module instead.

The shelve module allows you to tie a dictionary to a file, such that any chance to the dictionary, is automatically saved to the file. Thus, your file reading and writing code could be replaced by two lines:

import shelve



dbase = shelve.open("data.shf")

somehollis,

your code is basic, uncomplicated and easy to read and understand, all in the spirit of Python! Very nice!

My suggested improvement would be more on the user's side. When you add or remove an item, it would nice to add a list of recorded persons, so a mistake in typing the name could be avoided. Maybe list the recorded persons by number, so you only have to enter that number.

Thanks for the suggestions, guys (especially Averos for pointing at some things that I didn't yet know were possible and Dietrich for the insight on the user side). Now I have something play with.

To Booooze, I just thought I'd point out that in Python, indenting is actually a required part of the syntax. Improper indents lead to error messages. A friend of mine suggested Python as a starting point for learning programming because it would teach some good coding habits, and I believe this is (in part, at least) what he was referring to.

One I missed. The following code for choice 5:

         for x in dbase.keys():

            print "%s:" % (x),

            for y in dbase[x]:

                if dbase[x].index(y) != len(dbase[x]) -1:

                    print "%s," % (y),

                    

                else:

                    print y

            print""

Might be better written:

for key, list in dbase.items():

    print key + ":"

    print ",".join(list)

print

Quote:

Originally Posted by somehollis

To Booooze, I just thought I'd point out that in Python, indenting is actually a required part of the syntax. Improper indents lead to error messages. A friend of mine suggested Python as a starting point for learning programming because it would teach some good coding habits, and I believe this is (in part, at least) what he was referring to.

Like I said, don't know much about Python (but now I know that thanks!)
Good language to start with and it seems liek your taking it slowly, which is good. You don't want to be one of those people that want to dive into 3d Games right away with no experience. :p

Quote:

Originally Posted by Booooze

Yeah... learning all this is more about satisfying my curiousity than striving to write the "best.program.ever." I have ideas for stuff I'd like to write someday down the line, but it's mostly medical record type software since I don't like the stuff I'm forced to use in tracking my patients now.

Games are fun to play and all, but I'm happy to leave the making thereof to other people... :D