I missed this post first time around.
First a comment on efficiency in CGI scripts: don't worry about it. In a CGI program like this the bulk of the time is going to be spent loading Python, loading the script, and compiling it to bytecodes. Unless you have a loop in you script that is doing heavy processing then nothing you can do to the code will make any noticeable difference. If execution time is an issue for you then look into one of the alternatives to CGI where Python is running full time so does not need to be loaded: mod_python, mod_cgi, FastCGI, WebWare, Zope etc.
Now down to the code:
name = name.lower()
Don't bother with the isalpha test - name = name.lower()
is all you need, since it will make all alpha characters lowercase and leave the rest. The test for isalpha will return False if any
character in the string is non-alpha, including spaces. So "John Doe" will not be made lowercase since it will fail the test.
print "Enter a valid area code"
I presume by area code that you mean Zip code? The UK and many other countries have an alphanumeric area code, e.g. "AB12 CD34", so your test will exclude those from outside of america. It will also accept as valid any string that has any non-alpha characters (e.g. '£$%&**'), as well the empty string. If you want to check that a string only contains 0-9 then use str.isdigit().
3) ditto for all the phone number tests. But also see my rant on checking phone numbers here
except Exception, e:
print "please input your", e
What will be in the exception object e? Why do you assume it will be a string saying what is missing?
f = file(r"C:\Program Files\Abyss Web Server\htdocs\log.txt", "r+")
length = len(f.read())
open is intialised to 0 at the start of the script, so will always be set to 1 at this point. What is it supposed to represent?
You are also reading in the entire file just to seek to the end - I take back what I said about not needing to optimise. You can seek directly to the end of the file with f.seek(0,2)
. You can also open the file for appending in which case the seek to the end happens automatically.
6) It looks like you are writing to the log file that the server is using. THIS IS VERY DANGEROUS. The server may be be in the process of writing to the file, in which case the call to write() may fail, or the server may write to the file between your seek and your write statements, which will result in information getting overwritten. It sounds from your description that this may be what is happening. Create your own log file and write to that.
except Exception, e:
you have enabled cgitb to display any exceptions in a nice HTML friendly way, but then you catch all exceptions and display the same information in plain text. Remove the last try/except block and let cgitb do its work.
Finally a note on good practice: if you separate out the CGI part and put the data handling into a separate function then you can easily test the code either interactively or with unittest. e.g.
def checkAndSaveFormData(name, exp, area_code, phone_prefix,...):
if __name__ == '__main__':
name = cgi.form['name'].value
exp = cgi.form['exp'].value
Dave - The Developers' Coach