Programming Forums
User Name Password Register
 

RSS Feed
FORUM INDEX | TODAY'S POSTS | UNANSWERED THREADS | ADVANCED SEARCH

Reply
 
Thread Tools Display Modes
Old Jun 11th, 2006, 11:51 PM   #1
somehollis
Programmer
 
somehollis's Avatar
 
Join Date: May 2006
Location: Memphis, TN
Posts: 31
Rep Power: 0 somehollis is on a distinguished road
Send a message via AIM to somehollis
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..."
somehollis is offline   Reply With Quote
Old Jun 12th, 2006, 12:12 AM   #2
Booooze
Expert Programmer
 
Booooze's Avatar
 
Join Date: Mar 2006
Location: Igloo
Posts: 710
Rep Power: 3 Booooze is on a distinguished road
Send a message via MSN to Booooze
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
Booooze is offline   Reply With Quote
Old Jun 12th, 2006, 4:07 AM   #3
demon101
Hobbyist Programmer
 
demon101's Avatar
 
Join Date: Mar 2006
Location: westboro, ohio
Posts: 160
Rep Power: 0 demon101 is an unknown quantity at this point
Send a message via Yahoo to demon101
like booooze said that is a very nice clean looking code. i think that is great program you made just learning python.
__________________
Demon101 Production's

Code Forums
demon101 is offline   Reply With Quote
Old Jun 12th, 2006, 5:02 AM   #4
Arevos
Programming Guru
 
Arevos's Avatar
 
Join Date: Aug 2005
Location: England
Posts: 1,499
Rep Power: 5 Arevos is on a distinguished road
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")
Arevos is offline   Reply With Quote
Old Jun 12th, 2006, 12:11 PM   #5
Dietrich
Professional Programmer
 
Dietrich's Avatar
 
Join Date: Feb 2005
Posts: 434
Rep Power: 4 Dietrich is on a distinguished road
Smile

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.
__________________
I looked it up on the Intergnats!
Dietrich is offline   Reply With Quote
Old Jun 12th, 2006, 1:24 PM   #6
somehollis
Programmer
 
somehollis's Avatar
 
Join Date: May 2006
Location: Memphis, TN
Posts: 31
Rep Power: 0 somehollis is on a distinguished road
Send a message via AIM to somehollis
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.
somehollis is offline   Reply With Quote
Old Jun 12th, 2006, 2:19 PM   #7
Arevos
Programming Guru
 
Arevos's Avatar
 
Join Date: Aug 2005
Location: England
Posts: 1,499
Rep Power: 5 Arevos is on a distinguished road
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
Arevos is offline   Reply With Quote
Old Jun 12th, 2006, 2:41 PM   #8
Booooze
Expert Programmer
 
Booooze's Avatar
 
Join Date: Mar 2006
Location: Igloo
Posts: 710
Rep Power: 3 Booooze is on a distinguished road
Send a message via MSN to Booooze
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
Booooze is offline   Reply With Quote
Old Jun 12th, 2006, 6:18 PM   #9
somehollis
Programmer
 
somehollis's Avatar
 
Join Date: May 2006
Location: Memphis, TN
Posts: 31
Rep Power: 0 somehollis is on a distinguished road
Send a message via AIM to somehollis
Quote:
Originally Posted by Booooze
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
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...
somehollis is offline   Reply With Quote
Reply

Bookmarks

« Previous Thread in Forum | Next Thread in Forum »

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Forum Jump




DaniWeb IT Discussion Community
All times are GMT -5. The time now is 2:59 AM.

Powered by vBulletin® Version 3.7.0, Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
Copyright ©2007 DaniWeb® LLC