Friday, October 19, 2007

Assert.isTrue(false) never works

In case you were wondering, this pretty much throws an exception every time. Although, I one of my first mentors often tried to blame stray electrons on some bugs so, I guess if he was right, there could be times when this does work. But I wouldn't put money on it.

I ran across this particular piece of code when it showed up in a bug report recently. It came at the end of a if-then-else chain as the else that the author didn't expect to ever run. Well, this one unfortunate user found some way to make it run. Maybe it was a stray electron, or maybe it was just a corrupt preference store. We'll have to see.

But this did bring up another of my mantras for software development. Don't throw an exception that you don't plan on catching. Sometime we use exceptions as a crutch. If we get to some point where we can't figure out how to continue, we just throw an exception and don't need to worry about it any more. At least until the exception bubbles up to the UI and the poor user has to deal with it.

It's a tough job to put in all the possible error handling for a given situation. If you're building a safety critical system, you get a whole boat load of funding to pay for that effort. But when you're building a software development tool, you often feel that you don't have the time to work on code that you don't think will ever run. But it really does pay off in the end. When you have hundreds of thousands of users of your software, there's a pretty good chance one of them will run into a stray electron or two.

5 comments:

  1. Reminds me of a paper from a while back where the Java type-security mechanism was circumvented by using stray electrons to induce physical memory errors:
    www.cs.princeton.edu/sip/pub/memerr.pdf

    ReplyDelete
  2. Problem is: If they think an else-branch will never be reached, most people simply will not write the branch at all.

    So is throwing an exception just the better of two evils? At least it follows two principles of an Pragmatic Programmer: "Use Assertions to Prevent the Impossible" and "Crash Early". Actually there are many programs where it is good enough to throw an exception in branches which you think will never be reached.

    But: If you are programming something like a framework, you have to document what danger of exceptions exists where - even if they are unchecked. And if you are programming a UI you have to catch exceptions in a most user friendly way.

    ReplyDelete
  3. I agree that assertions shouldn't be used lazily, and they definitely aren't a substitute for proper error handling.

    Assertions are there to help you write bug-free robust code if done right. They help you catch bugs during development by alerting you right at the source of the problem rather than letting something erroneous manifest itself later. Its my understanding that they should be turned off when the code is running in production. Since you can't assume that asserts are going to execute you shouldn't use them in place of proper error handling.

    There are programming languages that make runtime assertions part of the overall language philosophy, its called design by contract and can be found in languages like Eiffel and D. The idea is to write code that enforces preconditions, postconditions and class and loop invariants. This helps make sure that all the different components of the software are behaving correctly and obeying each others interface contracts.

    Anyway, I guess its debatable, personally I use assertions rather sparingly. I used to use them a lot more in the past but since coming to CDT its dropped off.

    ReplyDelete
  4. We use assertion in a design by contract style. And we mostly leave them turned on in production, we just don't let the AssertionError propagate to the user.

    Why do we leave them on? Well, they shouldn't trigger in a production environment. But we all know that programming errors are made and if a assertions fails, it is better to fail there (see "Crash early") than to keep an running with broken assumptions.

    ReplyDelete
  5. The only reason an assertion should fail is if there is a bug. I haven't actually looked at the code in question, but I bet that if the assertion hadn't been there the code probably would have just blown up somewhere else. And maybe then it wouldn't have been so obvious that the problem was.

    I used to use design by contract style as well. One thing that I used to do all the time was enforce preconditions on public methods and throw IllegalArgumentException if violated. Precondition enforcement on constructor methods helps prevent you from creating an object that has invalid state. For example I recently found a bug in my code where I was creating a name node object but I was passing null to the constructor. It ended up causing an NPE in the indexer and I had to use the debugger to track down the problem. If the constructor had thrown an exception when I passed it null I would have instantly known there was a bug in my code and I would have known exactly what caused it and where to fix it. Instead I was able to create an object that was a ticking time bomb and it exploded far away from the real location of the cause of the bug. DBC and the use of assertions is a great method for writing quality software because it detects bugs early, during development instead of after the product shipped.

    ReplyDelete