![]() |
|
![]() |
|
|
Thread Tools | Display Modes |
|
|
#1 |
|
Programmer
|
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..." |
|
|
|
|
|
#2 |
|
Expert Programmer
|
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 ![]() |
|
|
|
|
|
#3 |
|
Hobbyist Programmer
|
like booooze said that is a very nice clean looking code. i think that is great program you made just learning python.
|
|
|
|
|
|
#4 | |
|
Programming Guru
![]() Join Date: Aug 2005
Location: England
Posts: 1,499
Rep Power: 4
![]() |
Quote:
try:
file = open("data.txt")
for line in file:
if line == "": break
values = line.split(',')
dbase[values[-2]] = values[:-2]
finally:
file.close()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()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 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") |
|
|
|
|
|
|
#5 |
|
Professional Programmer
Join Date: Feb 2005
Posts: 434
Rep Power: 4
![]() |
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! |
|
|
|
|
|
#6 |
|
Programmer
|
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. |
|
|
|
|
|
#7 |
|
Programming Guru
![]() Join Date: Aug 2005
Location: England
Posts: 1,499
Rep Power: 4
![]() |
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""for key, list in dbase.items():
print key + ":"
print ",".join(list)
print |
|
|
|
|
|
#8 | |
|
Expert Programmer
|
Quote:
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 |
|
|
|
|
|
|
#9 | |
|
Programmer
|
Quote:
Games are fun to play and all, but I'm happy to leave the making thereof to other people... ![]() |
|
|
|
|
![]() |
| Bookmarks |
| Currently Active Users Viewing This Thread: 1 (0 members and 1 guests) | |
| Thread Tools | |
| Display Modes | |
|
|