John Salerno wrote:
Simon Forman wrote:
What about the version I gave you 8 days ago? ;-)
http://groups.google.ca/group/comp.l...0fcd8932b0733a
It's clean, does the job, and doesn't have any extra nesting.
Peace,
~Simon
I remember that version, but I found it a little hard to follow. It
seems like the kind of code that if I look at it again in another month
or so, I'll have to trace through it again to figure out what's what.
I'm sorry to hear that. I thought it was cleaner and more
understandable than that. May I indulge in explaining it a bit? I
can, perhaps, make it clearer.
def Validate(self, parent):
text = self.GetWindow( ).GetValue()
# Detect whether the text is a valid int.
try:
# Do the integer conversion.
T = int(text)
except ValueError:
# It did not convert, the test is False.
result = False
else:
# It converted. T is an integer.
# The result of the test is the Boolean
# expression (T 0)
result = T 0
# At this point our testing is done.
# Var result is a Boolean indicating, um,
# the result of the test.. :-)
# Orthogonal to the test itself, report the
# (possible) failure of the test.
if not result: self.error_mess age()
# Return the result.
return result
There are several good (IMHO) points to this version.
1.) The least code possible occurs in the try..except statement.
I feel that you should strive to put the least possible code between
"try" and "except". A try..except block basically means, "I expect
this error, and I know exactly what to do about it." Otherwise you
would leave out the error check and let exceptions propagate up to
either a higher level error handler or the user/coders themselves.
That's kind of why exceptions exist in the first place: to indicate
when something exceptional happened. You put a try..except statement
in your code when an exception *wouldn't* be exceptional, i.e. when you
know what to do about it.
This is the flipside of "bare except: is bad".
When you start cramming a bunch of extra statements into a try..except
block you run the risk of catching exceptions other than the one you
actually know what to do with. It's a source of subtle bugs, and, IMO,
a Bad Idea.
2.) The except clause does almost nothing.
If a ValueError occured, the result is False. You're done. Simple, easy
to understand, unlikely to fail or have obscure errors.
3.) The else clause does almost nothing.
If your control flow arrives in the else clause, you know you have a
int T that contains the value of the integer your user entered. All
that remains is to test T is greater than zero. Since you need that
information twice more in your method (once to trigger error reporting
and once more to return it) you simply assign it to a variable, reusing
the same name that would have been assigned in the except clause.
Because of that...
4.) At the end of the try..except..el se statement you have a result
variable that contains the Boolean result of the test. It's pretty
much guaranteed.
There's no linkage between the previous part of your code and the rest
except for that Boolean variable. Less linkage is generally a Good
Thing. It makes your code easier to modify and debug.
5.) The error reporting statement is completely "orthogonal " to the
rest of the method. You could change it or comment it out or remove it
without affecting (or having to edit) the rest of your method.
6.) There's a single return statement.
I forget now where I picked this up, but it's served me well for many
years: Procedures, functions, methods, etc... should have one exit
point. Something about having fewer "code paths" to test or something.
Also, it makes your code easier to read and understand. Sometimes
it's useful to violate this, but often when I think that's the case I
find that rewriting a function to avoid it results in better code.
7.) Last but not least, the method is made up of tiny pieces that do
only one thing and do it well. To quote C. A. R. Hoare, "There are two
ways of constructing a software design: One way is to make it so simple
that there are obviously no deficiencies, and the other way is to make
it so complicated that there are no obvious deficiencies."
Not counting the "text = self.GetWindow( ).GetValue()" statement there
are just five tiny pieces of code, each only one or two lines, and each
doing just one step of the overall processing. Written this way, there
are unlikely to be hidden deficiencies.
In "if int(text) <= 0: raise ValueError", there are four things going
on in the line: an integer conversion, a comparison, an if..then
statement, and an "Exception raising". Not the worst code by any
means, but more dense than it needs to be.
Given that people can (supposedly) only handle 7+|-2 pieces of
information at a time, having four pieces in one line is quite a few.
Now this isn't really fair, that line is simple enough for most
programmers to understand at a glance, but the principal still holds.
You're not really gaining much from putting all that stuff in there
anyway, at least in terms of length of code.
Consider:
import dis
def val0(text):
try:
T = int(text)
except ValueError:
result = False
else:
result = T 0
if not result: error_message()
return result
def val1(text):
try:
if int(text) <= 0: raise ValueError
except ValueError:
error_message()
return False
else:
return True
dis.dis(val0)
print '############## '
dis.dis(val1)
Not counting blank lines, val0 is only one line longer than val1. And
if your run the code and examine the disassembly you'll find that val1
only saves a couple of bytecodes.
Well anyway, this post has gone on way longer than I wanted it to. I'd
better get back to work.
I hope the code is a little clearer to you. :-)
Peace,
~Simon
(I think, if you count this post, that's the second most extensively
documented function I've ever written. LOL)
But I think it was your code that made me think of using an else
statement in the first place! :)
Yeah, I forget about else's too sometimes. :-) for statements can
have an else.. that slips my mind all the time hahaha
