Will's blog

purpose: Will Kahn-Greene's blog of Python, Linux, random content, PyBlosxom, Miro, and other projects mixed in there ad hoc, half-baked, and with a twist of lemon

Tue, 30 Sep 2008

Bad code: Part 1

If you're writing code like this:

try:
   foo = somevar.getBlah()["xyz"].split(".")[-1].decode("ascii", "replace")
except:
   pass

Please stop! You're killing the rain forest!

Posted by Sidharth Shah on Tue Sep 30 13:19:42 2008
So I am trying to understand this, is using multiple operations on a single line bad or using try and pass bad. Or both?


Posted by jt on Tue Sep 30 13:22:13 2008
While it might feel good to call this "Bad Code", you would contribute more to the world if you would (1) explain why it is bad code, and then (2) show code with equivalent functionality that is not bad.  Kthnksbye!


Posted by will on Tue Sep 30 13:42:45 2008
I didn't write this for feel-good purposes--it's been a frustrating day.

I don't think I have an obligation after writing this blog entry to teach nor do I think I can adequately cover the issues involved in a single blog entry.

You could use it as a litmus test: if you have no clue why the above code is bad code, then you're probably deficient in understanding exception handling and chaining in respects to code brittleness.  You'd learn a lot more by researching and writing your own blog entry about why the above is bad and how to make it better than by reading a few paragraphs I threw together.


Posted by Gryc Ueusp on Tue Sep 30 13:56:45 2008
You obviously know why it's bad, so tell us why.  Otherwise, you're just whining.


Posted by Jeff Forcie on Tue Sep 30 14:11:36 2008
Sidharth, it's "both", although not in absolutes by any means.

Multiple operations per line is obviously a fine tool to use, but taking it to the extreme that Will's example does is taking it a bit too far; I can see at least 5 or 6 potential points of failure in that line, meaning that its chances of failure at any given point are pretty darn high. By the end of the line you're practically guaranteed a failure unless each and every one of the prior assumptions just happens to be correct.

Then, pairing that with an except: pass, which is almost NEVER a good idea in any circumstances (it causes a silent failure; at the very least, you want a high level catchall 'except' that logs uncaught errors, if nothing else, and this prevents such behavior) means that you're mating a high failure potential with a silent error. This is Not A Good Thing! :)

I think of this blog post as the Python equivalent of complaining that your PHP using co-workers are doing stuff like calling a database query with an unescaped GET parameter...


Posted by rgz on Tue Sep 30 14:38:16 2008
Wow don't get so defensive, you don't have to explain yourself, you just will be understood less.

I understand your frustration, but come on, it's fun and it does come in handy in throw away scripts. The following is gonna make you cry ;)

Once I wanted the href of all hyperlinks that contain an image with alt text that start with "Exhibit", here we go:


import urllib2, BeautifulSoup
def filtry(f):
def _(args, *kwargs):
  try:
  return f(args, *kwargs)
  except:
  return False
return _

@filtry
def myfilter(tag):
return tag.contents[0]['alt'].startswith("Exhibit")

hrefs = [tag['href'] for tag in BeautifulSoup.BeautifulSoup(urllib2.urlopen("foo").read()).findAll(myfilter)]


Actually, filtry is properly implemented in my batbelt.


Posted by Sidharth Shah on Tue Sep 30 15:50:22 2008
Thanks jeff, @jt you are right I don't have idea of how exceptions work, I will do some research.


Posted by tshirtman on Wed Oct 1 03:33:27 2008
Well, I do this king of things on a daily basis,, I try to avoid the except - pass, althought, but, the oneliner is not so bad, (or at least I did worse by a good lenght).


Posted by will on Wed Oct 1 08:24:18 2008
I've gotten some really awful comments from this blog post which I've deleted.  I'm really surprised anyone has taken the time to flame me on this--it's a silly kvetching blog post I threw together after spending a day tracking down problems caused by bad code.  I even managed to squeeze in a reference to a show called the IT Crowd--it's funny!  That's all it is.  If you knew me, you wouldn't have thought twice about it.

My comment shouldn't be read as a damnation of anyone--it's an honest perspective of the situation.  We're all deficient in things.  I'm not sure why telling people to research the issues seems unpopular--if you're not continually researching and learning new things, how do you expect to keep up with the rapidly changing world of software development?

Jeff's covered some the issues, but I think there's a lot more going on than that.  Instead of flaming me, I encourage you readers to pick out something wrong with that bad code and write about it.  You can do that here or in your own blog post.

Here's one thing: the above code catches all exceptions including SystemExit and KeyboardInterrupt.  Thus this bit of code has the unintended consequence of catching and ignoring ctrl-c and system exit.  This can result in hung processes.  It's one of the reasons for the changes in Python 2.5 to the Exception hierarchy.  Definitely worth reading about if you don't know that already.


Posted by Giacomo on Thu Oct 2 09:37:27 2008
I personally would have said that you're going to have at least three completely different types of exceptions raised there ("no key in dict", AttributeError, anything related to text encoding), which is always bad to put together in one clause, plus the nullification of any exception which might be raised by the method, plus the obvious uselessness of "pass"... but I'd never have thought of Ctrl-C, probably because mostly of my work is not on desktop apps :)

(maybe just adding "what do you think is wrong about this?" would have spared you the flames...)


Please keep comments appropriate. I reserve the right to remove anonymous comments, flames, spammy, inappropriate, and other comments that I deem to be worth removing.

Note: New comments get placed in a "draft" status and will NOT show up on the site until I explicitly approve it. Usually that happens within 24 hours, but sometimes I go away and it takes a day or two.

Note 2: There is now a preview button for those of you who want to see a preview! However, it doesn't quite work the way you'd think it should work. I'll look into adjusting it some day.

Note 3: If you can't for some reason post a comment, send me an email: willg at bluesock dot org.

Your name:


Your e-mail address (this doesn't get displayed to anyone--I use it to contact you if there are issues):


URL of your website (optional):


Comment:


Yes, I am a human!

pyblosxom::2.0 dev

All contents Copyright 1996 to 2008 Will Guaraldi.
Creative Commons License
This work is licensed under a Creative Commons Attribution-Noncommercial-Share Alike 3.0 United States License.