The Zen of Doing It Wrong
May 6th, 2009
A coworker unearthed this little treasure today… I think it’s a vestigial structure to assist Diaper (anti)Pattern treatment during testing or debugging, but it’s still gross to see it in real, live production code. (Oddly enough, I couldn’t find a good “Diaper Pattern” link whilst Googling about–surely I’m not the only one who uses this term?) Anyway, without further ado:
try: os.remove(filename) except: raise pass try: os.remove(os.path.join(TMP,'out-%s' % base_filename)) except: raise pass try: os.remove(os.path.join(TMP,'properties_%s.lock' % brandid_human)) except: raise pass
If an exception is raised, raise an exception… It has a certain zenlike beauty to its awfulness. The use of the Diaper Pattern is bad enough, but this guarantees blowouts!
I don’t know who coined the phrase, but Mike Robellard and I used the phrase “diaper pattern” a lot when we were working on the CRT system.
I still use the phrase today; mostly when I catch way-too-broad except clauses.
I love the inclusion of the ‘pass’ statement as well. It just teases the program, letting it see that it is only one statement away from blissfully continuing on in ignorance of all errors. “None shall pass!”
@Christian Wyglendowski
Instead of the “none shall pass” Black Knight, I’m more envisioning Gandalf the Grey catching exceptions at the Bridge of Khazad-Dum, declaring “You *cannot* pass!”
hi,
I think I can explain the reason for this ugly code… I think
I bet the code at some point had some debugging code in there. So there was a problem with the file operations perhaps.
These try: raise: pairs allow you to quickly add some debugging code in there.
Then when the code was finished, the programmer left it in there in case they needed to do some more debugging later. So it was never cleaned up.
The pass were left in case the raise was to be removed… as I expect for the same reason it was left in.
Just some guesses… maybe I’ve done similar ugly things in the past, and that’s why I might have done this.
I guess in this case, it’s similar to using these other ugly things…
# easily disable a block by changing one character.
if 0:
do_stuff()
if 1:
do_stuff()
// allow putting more than one thing in the if/else blocks…
// if you need to later.
// Since C doesn’t require {} for single statements.
if(some_condition) {
do_one_thing()
} else {
do_one_thing()
}
Leaving ugly stuff like that in production code, lets you try and fix things quicker if needed.
cu,
at least a single try-except block like this could probably be easily found in my code. and actually, other than being more code than necessary, i don’t think there’s a big problem with this. typically, i comment out the raise to do some testing. but i agreee, if production code means, all testing has been done – or is automated – superfluous code should be removed.
So, what’s a Diaper Pattern?