[Tutor] Simple Python Address Book (Advice welcome!)

[Alan Gauld]

On 17/06/12 02:45, mariocatch wrote:

> I'm essentially asking if someone wouldn't mind taking a look at what I
> have so far, and giving some advice on some of my weak areas,

Welcome, I've inserted a few comments below.
In general it's OK although personally I'd probably put the email 
validation in a short helper function (including the regex definition 
etc) just to tidy it up and avoid one level of indentation. (Its also 
potentially reusable so I'd probably put that function in its own module 
too...

I'd probably also put all the splitting logic into LoadAddresses so that 
what I get back is the actual address dictionary rather than the raw 
list of lines.

That would clean up main() quite a bit.

Also it's conventional to make function names start with lowerCase. 
UpperCase names usually imply it's a class.

Finally, I'd put all the functions above main() rather than having the 
lonely looking RecordAddresses() at the end.

> import re # email validation
> import textwrap
> # forward declarations
> addressBookPath = r'C:\temp\addresses.txt'
> addressBook = {} # address book held in memory before dumped to file
> emailFormatRegex = r'(\w[\w]*)@([\w]+\.[\w]+)'
> recordDelimiter = ' | ' # split name | email in file
> def LoadAddresses():
>      """ Loads all addresses from file and places
>      them in tuple in form (bool, list), where the list is each
>      row from the file (a person's name | email). """
>      success, lines = (False, [])
>      try:
>          f = open(addressBookPath, 'r')
>          lines = f.readlines()
>          f.close()
>          success, lines = (True, lines)
>      except IOError as e:
>          print 'Error opening address book: ', e
>      return (success, lines)

The latest style preference in Python is to use with statements for file 
handling, so that would become:

try:
   with f as open(.....):
      success,lines = True, f.readlines()
except IOError as e:
   print ...
return (success,lines)


'with' guarantees file closure automatically.


> def main():

>      (success, lines) = LoadAddresses()
>      if not success:
>          shouldMakeNewBook = raw_input(textwrap.fill("""You do not have
> an address book yet.
>                                                  Would you like to
> create one?"""))
>          if shouldMakeNewBook in ('y', 'ye', 'yes'):
>              f = open(addressBookPath, 'w')
>              f.close()
>              print 'New address book created.', addressBookPath
>          else:
>              print 'Exiting...'
>              return
>      else:
>          # now that we have the file loaded into memory,
>          #  fill out the addressbook from file
>          for line in lines:
>              splitStr = line.split(recordDelimiter)
>              addressBook[splitStr[0]] = splitStr[-1]
>      # main input loop (break with 'q' or 'quit' during input)
>      while True:
>          newPersonNameInput = raw_input('Enter new person\'s name:
> (q/quit to stop):')
>          if newPersonNameInput.lower() in ('q', 'quit'):
>              break
>          addressBook[newPersonNameInput] = newPersonNameInput
>          while True: # loop until email is in valid format (x at y.z
> <mailto:x at y.z>)
>              newPersonEmailInput = raw_input('Enter new person\'s email:
> (q/quit to stop):')
>              match = re.search(emailFormatRegex, newPersonEmailInput)
>              if not match:
>                  print 'email validation failed... try again.'
>                  continue

continue is not strictly needed here, but does make the intent clear.


>              else:
>                  addressBook[newPersonNameInput] = newPersonEmailInput
>                  break # success
>      RecordAddresses()
>      print addressBook
> def RecordAddresses():
>      """ Writes out each address to the file in the form of name |
> email. """
>      f = open(addressBookPath, 'w')
>      for k, v in sorted(addressBook.items()):

Should have a try in case the file fails to open.
Also slightly more pythonic to do

for k,v in open(....):

There is no point in sorting the data if you are going to read it into a 
dictionary, Python's dictionaries are unsorted. (Unless you plan on 
reading the file manually - in a text editor say)

>          #is there a better way to do this without placing a newline
> after each row?
>          #it's causing multiple line separations when writing back out
> to file (because each
>          #  time we finish reading, it ends with a trailing newline).
>          f.write("{0}{1}{2}\n".format(k, recordDelimiter, v))

Sorry, you have to add a newline when writing to the file and strip it 
off when reading. Its just how it is...

-- 
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/