[Tutor] Objectifying code
Magnus Lycka
magnus@thinkware.se
Fri Jan 24 13:44:02 2003
At 07:38 2003-01-24 -0800, Sean 'Shaleh' Perry wrote:
>Dunno, after reading the code I am not but so sure objects are the right
>solution.
To be honest, I didn't bother reading through the code well enough
to understand what it did. This is partly because I'm in a hurry,
and partly because it's written in a way that is difficult to read.
I would suggest that you use the normal convention of python code,
instead of writing something looking like an overgrown bat-file. ;)
In other words, use the standard idiom of
if __name__ == '__main__':
main()
in the end, make a main() function that captures the main
abstractions of the program, so that people can read that function
(which is hopefully note more than a screenful) and understand
what it's all about. You don't need classes for this, you can use
well named functions.
I'm sure that it's reasonable to divide most of the main code
in chunks that logacally form an abstraction, or a step in your
processing if you like. Hopefully there won't be too many vaiables
that go beyond the boundries of these blocks. If there aren't
terribly many shared variables, it's not very difficult to make
these chunks into separate functions.
Actually, if there are a lot of shared data, that might be a
reason to use one or several classes instead, since class instances
can remember their data (in attributes) between method calls, but
remember that a class should capture one abstraction.
I would also suggest that you remove all the big strings from the
main body of the code. They completely mess up the readability that
python normally provides. Let's have a look at a piece of code:
elif 'edit' in Args: # Edit a record
for entry in lankaDB:
if entry['firstname'] + entry['lastname'] == Args['edit']:
print """<h1>Editing %s %s's record</h1>
<form method=get action="%s">
<p><input name="submitchange" type="hidden" value="%s%s">
First name: <input name="firstname" type="text" size="30" value="%s"><br>
Last name: <input name="lastname" type="text" size="30" value="%s"><br>
Phone: <input name="phone" type="text" size="30" value="%s"><br>
E-mail: <input name="email" type="text" size="30" value="%s"><br>
Address: <textarea name="address" rows="4" cols="30">%s</textarea><br>
Comments: <textarea name="comments" rows="6" cols="30">%s</textarea><br>
Password: <input name="password" type="password" size="10">
(must match old password)<br>
<input type="submit" value="Submit changes">
</p></form>""" % (entry['firstname'], entry['lastname'], cgiaddr,
entry['firstname'], entry['lastname'], entry['firstname'],
entry['lastname'], entry['phone'], entry['email'],
entry['address'], entry['comments'])
print """<hr><b>Or</b>, change %s %s's password:
<form method=get action="%s">
<p><input name="changepassword" type="hidden" value="%s%s">
Old password: <input name="oldpassword" type="password" size="10"><br>
New password: <input name="newpassword" type="password" size="10"><br>
New password: <input name="newpassword2" type="password" size="10">
(just to be sure we get it right)<br>
<input type="submit" value="Submit new password">
</p></form>""" % (entry['firstname'], entry['lastname'], cgiaddr,
entry['firstname'], entry['lastname'])
print """<hr><b>Or</b>, delete:
<form method=get action="%s">
<p><input name="delete" type="hidden" value="%s%s">
Password: <input name="password" type="password" size="10">
(must match old password)<br>
<input type="submit" value="Delete %s %s">
</p></form>""" % (cgiaddr, entry['firstname'], entry['lastname'],
entry['firstname'], entry['lastname'])
Here, it's completely hopeless to follow the flow of the code.
At least for me. Has the if statement ended yet? The for-loop? I
don't know, I can't make myself try to follow this without giving
it an overhaul.
This big string I'd make into a global variable. Not because it's
so global in nature, but because it's a small program and I want
this big chunk of HTML away from the flow of the code. Maybe I'd
even put it in a separate file and import it?
recordEditForm = """
<h1>Editing %(firstname)s %)(lastname)s's record</h1>
<form method=get action="%(cgiaddr)s">
<p>
<input name="submitchange" type="hidden"
value="%(firstname)s%(lastname)s">
First name: <input name="firstname" type="text"
size="30" value="%(firstname)s"><br>
Last name: <input name="lastname" type="text"
size="30" value="%(lastname)s"><br>
Phone: <input name="phone" type="text"
size="30" value="%(phone)s"><br>
E-mail: <input name="email" type="text"
size="30" value="%(email)s"><br>
Address: <textarea name="address" rows="4"
cols="30">%(address)s</textarea><br>
Comments: <textarea name="comments" rows="6"
cols="30">%(comments)s</textarea><br>
Password: <input name="password" type="password" size="10">
(must match old password)<br>
<input type="submit" value="Submit changes">
</p>
</form>
<hr>
<b>Or</b>, change %(firstname)s %(lastname)s's password:
<form method=get action="%(cgiaddr)s">
<p>
<input name="changepassword" type="hidden"
value="%(firstname)s%(lastname)s">
Old password: <input name="oldpassword"
type="password" size="10"><br>
New password: <input name="newpassword"
type="password" size="10"><br>
New password: <input name="newpassword2"
type="password" size="10">
(just to be sure we get it right)<br>
<input type="submit" value="Submit new password">
</p>
</form>
<hr>
<b>Or</b>, delete:
<form method=get action="%(cgiaddr)s">
<p>
<input name="delete" type="hidden" value="%(firstname)s%(lastname)s">
Password: <input name="password" type="password" size="10">
(must match old password)<br>
<input type="submit" value="Delete %(firstname)s %(lastname)s">
</p>
</form>"""
The only thing left in the part of the program where I cut out
this section would be:
elif 'edit' in Args:
# Edit a record
for entry in lankaDB:
if entry['firstname'] + entry['lastname'] == Args['edit']:
print recordEditForm % (entry + {'cgiaddr': cgiaddr})
This is easier to read, isn't it? We don't have to worry about HTML
form layout and program flow at the same time. The indentation is
helpful in suggesting a structure, and the use of string % dict instead
of string % tuple both cleans up the print statement and makes it
much easier to modify and review the HTML forms.
--
Magnus Lycka, Thinkware AB
Alvans vag 99, SE-907 50 UMEA, SWEDEN
phone: int+46 70 582 80 65, fax: int+46 70 612 80 65
http://www.thinkware.se/ mailto:magnus@thinkware.se