John Salerno wrote: > Simon Forman wrote: > > > What about the version I gave you 8 days ago? ;-) > > > > http://groups.google.ca/group/comp.lang.python/msg/a80fcd8932b0733a > > > > 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_message() # 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..else 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 -- http://mail.python.org/mailman/listinfo/python-list