Fog Creek Software
Discussion Board




Has Joel ever really coded in Java?

I usually find Joel's articles very entertaining and interesting.  Unfortunately I also have to add my criticisms about Joel's exception article.  I think his comments about the Java Exceptions indicate a lack of understanding of the Java platform.  C++ is well know for problems with its exception framework and I think Joel has erroneously lumped Java in that basket.

Java has 2 exception types: Checked Exceptions and Runtime Exceptions.  It also has Runtime Errors.

Checked Exceptions are declared in a method signature.  They are application or library specific exceptions which provide a context to any exceptional conditions which *are known at development time* that can occur.  When a method with a declared checked exception is used, the compiler ensures that the exception is handled by the calling code.  It is very visible in the code and the documentation.  The exception framework allows the caller to act appropriately when an exception occurs or else throw the exception to a higher level in the call stack where it can be handled intelligently.  Bad programmers can abuse this facility by swallowing exceptions (re-throwing a java.lang.Exception up the stack in place of the specific caught exception such as java.io.IOException).  Programs are intended to recover from checked exceptions.  An example is when a GUI file dialog tries to open a file which no longer exists.  An IOException is thrown which can be caught at the appropriate level in the application to allow an alert to be displayed to the user informing them of the problem and asking them to try again.  Code Reviews should stop any misuse of checked exceptions.  There is never a reason (except laziness) to misuse checked exceptions.

Runtime Exceptions on the other hand are not application or library specific.  What I mean by this is that their purpose is to react to programming or bad data errors *which were not expected to occur*. These type of exceptions are not checked at compile time because of that reason.  I actually believe Joel must have been talking about this class of exception because they are infact almost invisible.  An example is a divide by zero exception or an array index out of bounds exception.  These are programming or data bugs which were not supposed to occur.  If the application was of a nature where you would expect data problems which may give you a divide by zero error you should be aware of this at design time and you would specifically attempt to catch and deal with this exception.  However in general you would not want to have specific code to check *every* calculation you ever write for divide by zero errors.  It is this simple, they are design for use in the case when you have decided you are not prone to these exceptional conditions.  If you know you are then you are prepared for them and catch them.  If you know you are not then you shouldn't have to catch them and so you would not put the catch blocks in your code.  The case for writing your own Runtime Exceptions is not a strong one.  Their use should be restricted and a checked exception should always be favoured.  Again code reviews can stop misuse of these exceptions.

java.lang.Errors are the final throwable object.  These relate to the java platform system errors like OutOfMemory.  These are runtime and not checked by the compiler.  In this case there is very little you can do, its a bit like a core dump in C!

Joel, I can't help but feel a little bit of your credibility fading away when you talk about technical issues which appear out of your areas of expertise.  I hope that you will look at this area again and come to the conclusion that Java exceptions are not all that bad and a hell of lot better than spaghetti switch statements testing error values.

Donal McCarthy
Wednesday, October 15, 2003

As the *other* "Joel and Java" thread points out, Joel did code in Java in 1996. The Exception model was basically the same then as now, though.

> Joel, I can't help but feel a little bit of your credibility fading away when you talk about technical issues which appear out of your areas of expertise.

Agree.

Stick with the little digs (eg, how the Linux guys rewrote the firewall code *again*). They're funnier and they keep your foot out of your mouth.

Portabella
Wednesday, October 15, 2003

"It is very visible in the code and the documentation."

No it isn't.  The exception is specified in the "throws" list.  The invisibility of exceptions means that one cannot look at a block of code, eg:

try
{
  foo1( );
  foo2( );
  foo3( );
}
catch( NumberFormatException e )
{

}

And _easily_ tell whether it's foo1, foo2, or foo3 that throws the NumberFormatException.  Whether it's checked or not really makes no difference.

_It's still not readable_.  Unless you like looking up every word in every sentence you read in the dictionary.

smkr45
Wednesday, October 15, 2003

Is knowing which of those calls throws the exceptions really such a huge ordeal, smkr45?

When you wrote that code, you probably knew. If you forgot, just remove the catch and if your IDE doesn't suck, you'll instantly know which call throws the exception. You could also just use your non-sucky IDE to click on the call and bring pop up the method signature.

Gordo
Wednesday, October 15, 2003

> And _easily_ tell whether it's foo1, foo2, or foo3 that throws the NumberFormatException. 

It's not clear from the code that you *have* to know.

Maybe the intent is that if any of the methods fail, we execute the catch block -- in which case it's doing exactly what it should.

If we *do* need to know... well, it's pretty trivial to rewrite it, eh?

This is by no means silly either; I've seen plenty of database code where, programatically, we really don't care where it went wrong (and from a debugging perspective, the stack trace will tell us); all we need to do is clean up.

Portabella
Wednesday, October 15, 2003

oh yes you can tell which function call threw the exception
the printstacktrace method will tell you the line

the artist formerly known as prince
Wednesday, October 15, 2003

The reason NumberFormatExceptions arise is because of poor design.  Whatever method is throwing it is assuming that the string argument is in a certain format.  When the method is called the assumption is false, so it's not legal to call the method.  This indicates a logical problem with the code, not an exceptional condition.  It may be useful during development, but an exception like this should not even be possible in production code, if the code is well designed and well built.

That aside, if pinpointing failing methods in your objective, wrap each method call in a try/catch block.  You get the same basic behavior as the error code approach, both of which are a mess.

hohum
Wednesday, October 15, 2003

"When you wrote that code, you probably knew"

Here in the real world, I don't write all the code I have to read. :)

So you're both right and wrong at once, eh?

"This is by no means silly either; I've seen plenty of database code where, programatically, we really don't care where it went wrong (and from a debugging perspective, the stack trace will tell us); all we need to do is clean up. "

And I've seen plenty of code where I damn well do care where it went wrong, thankyouverymuch, (especially with lower level APIs), and I'd like to fix the actual problem!

We can "he said, she said" all day long, but I think in this respect status returns are clearly more flexible.  Don't like the return codes?  Throw something.  But retroactively turning exceptions into status returns is an ugly exercise.

"When the method is called the assumption is false, so it's not legal to call the method.  This indicates a logical problem with the code, not an exceptional condition"

It's not a logical problem with the code at all.  eg, there's the old idiom:

get input
while( input invalid )
{
  message to user: invalid input!
  get more input
}

Now, the boolean expression "input invalid" should not mean "I have to check for myself whether it's in a valid format."  Because that's stupid; if I'm going to check it, I may as well do the conversion, too!  So the conversion method should tell me when it's in a bad format, and it should preferably do it in a way less obnoxious than an exception, since the structure of my program is just fine, thanks.

smkr45
Wednesday, October 15, 2003

I'm confused.  Why is this bad...?

public void installSoftware()
{
  try
  {
    copyFile();  // throws IOException
    makeRegistryEntries();  // throws RegistryException
  }
  catch (IOException ioe)
  {
    System.out.println("Error copying files!");
  }
  catch (RegistryException rex)
  {
    System.out.println("Error modifying registry!");
  }



Russell

Russell Thackston
Wednesday, October 15, 2003

Russell: shouldn't you uncopy the files if that step failed?

Remember, we've agreed that "catch" should clean up.

Joel Spolsky
Wednesday, October 15, 2003

you could have both catches point to a method that cleans up registry

the artist formerly known as prince
Wednesday, October 15, 2003

which any well structured program would have anyway

the artist formerly known as prince
Wednesday, October 15, 2003

> And I've seen plenty of code where I damn well do care where it went wrong, thankyouverymuch, (especially with lower level APIs), and I'd like to fix the actual problem!

Then the code you posted is buggy (obviously).

However, your problem is so vaguely stated that it's difficult to say *what* you should do; it depends very much on what cleanup actions you want to happen should one of the methods fail.

It's not difficult to imagine a good design with exceptions solving the problem quite nicely -- if you just deign to tell us what the problem *is*. A sketch might be:

// Or use a try/catch block in this method for common
// cleanup
public void doStuff() throws MyException
{
    doFoo1();
    doFoo2();
    doFoo3();
}

public void doFoo1()
{
    try
  {
      foo1();
  }
  catch (NumberFormatException nfe)
  {
      // cleanup, etc
      throw new MyException();
  }
}

... you get the idea, so I won't belabor it.

Portabella
Wednesday, October 15, 2003

Ouch!

I had a "backOut()" method being called in catch blocks of the example, but I didn't want to overdo the example, so I took it out.  I'll know better next time.

But that still doesn't answer my question...  What's wrong with the code?


Russell

Russell Thackston
Wednesday, October 15, 2003

"Because that's stupid; if I'm going to check it, I may as well do the conversion, too!"

It's not stupid.  It's separation of concerns.  One part answers a question about the data and one part performs a transformation on the data - clearly different tasks.  Lumping them together is poor design if there are other ways to avoid it.  Practically speaking, there are times when you have no other alternatives for one reason or the other, but this isn't one of them.

hohum
Wednesday, October 15, 2003

hohum, I use NumberFormatException all the time, to inform the user that their data is not a number.

If the data is not user entered, using it may indicate a design problem as you say, but in my case the catch will inform the user of their mistake.

This exception is thrown by things like Integer.parseInt, which gives you an int from a String. It's usually a good idea to deal with this exception.

Gordo
Wednesday, October 15, 2003

Maybe I should be clearer about why it's a design problem.  In this instance, you are using an exception to signal whether or not something is formatted so that it can be interpretted as a number.  The reason that you are signalling it this way is because the parseInt function has been overloaded to provide two pieces of information:

1. whether the arg is formatted as expected, and
2. the numeric equivalent of the argument

from a single-valued function.  A better design would have two functions to do these things instead of one.  With this design, you don't compromise your ability to alert your user to his mistake - you can still ask the question "Can this string be interpretted as a number?" and take appropriate action if the answer is "no." 

Exceptions are not supposed to be used for returning values or answering questions (in production, at least). When you see this happening, you should look at your design.  There's a good chance that something is amiss.

hohum
Wednesday, October 15, 2003

The proper manner of handling RuntimeExceptions is not to write code to catch them.  The proper manner is to fix the code so they don't occur in the first place.

As far as knowing where the exception occurred in a series of calls ... most of the time you don't need to know which specific one it is.  If you do need to know, you can write try/catch around each one if you want to continue with the next foo() after the exception, or just use a variable that marks where you were:

try {
  x = 1;
  foo1();
  x = 2;
  foo2();
  x = 3;
  foo3();
}
catch (Exception e) {
  switch (x) {
    case 1:  handleProblem1(); break; // exception was foo1
    case 2:  handleProblem2(); break; // exception was foo2
    case 3:  handleProblem3(); break; // exception was foo3
}

T. Norman
Wednesday, October 15, 2003

Joel,

I think you are hinting toward the guarantee of exception safety.  Herb Sutter outlines concise definitions of exception safety guarantees in his book Exceptional C++ on page 38. 

The can make some guarantee about the level of exception safety without "uncopying" the files.  If you did "uncopy" the files then this code would be said to enforce the "strong guarantee." 

I agree with a lot of what is said on this site, and there are times I dislike the party line of the C++ minds on the topic (please see my discussion on handling stack overflow on comp.lang.c++.moderated), but it is my opinion that you can not discuss the state of exception handling in C++ with out reading Sutter.  I feel as strongly about this as you feel about Unicode encoding.

http://www.summitsage.com/

christopher baus (tahoe, nv)
Wednesday, October 15, 2003

"The proper manner is to fix the code so they don't occur in the first place."

I think that's a little pollyannaish.  What the "proper manner" is doesn't matter once your code has been released, unless you work at one of those companies with infinite time and resources.  Yes, the problem _should_ be fixed, but in the meantime it's nice not to totally bomb out and confuse/irritate your customers.

The proper manner is to handle error conditions gracefully, whether they're your fault or not, and write code that makes it easy to isolate and fix those that are programmer error.  Exceptions handle the first scenario, but ne'er the second.

smkr45
Wednesday, October 15, 2003

BTW,

I know this topic is on Java, but the originally article included comments regarding C++.  I think Herb's exception guarantees can be mapped to Java. 

christopher

christopher baus (tahoe, nv)
Wednesday, October 15, 2003

> write code that makes it easy to isolate and fix those that are programmer error. 

A plain old vanilla stack trace will tell you where the error occurred -- even in your original example. You can then "isolate and fix" it.

And you can handle the requirements perfectly well with exceptions -- you now have *2* examples in this thread.

So what's your point again?

Portabella
Wednesday, October 15, 2003

"Yes, the problem _should_ be fixed, but in the meantime it's nice not to totally bomb out and confuse/irritate your customers."

I didn't mean you NEVER catch RuntimeExceptions. I meant they should not be specifically caught, as in catch(ArrayOutOfBoundsException e).  They should be caught by a more general exception handler, preferably at a higher level, that logs the occurence and exits gracefully or moves on to another task.  Then you fix the problem ASAP.

T. Norman
Wednesday, October 15, 2003

I think the core issue is over-generalizing the solution to a problem (in this case the exception -vs- status codes issue) to a one solution-fits-all thing.

Some people are sold on exceptions, so they use it all the time, even when it is not practical.

If you have written lots of Win32 API - based code, you will find that structuring code to use exceptions takes a lot of extra effort. It is more natural and straightforward to check the status return code directly rather than wrapping it around an exception and throwing it back to the user. Yes, you could wrap it around an exception, but is it worth it?

I believe Joel's point is to use whatever is most practical. Ex. for my Win32-based code, I use return status codes and hungarian notation. For my C# CLR-based code, I use exceptions and pascal casing. Use the most straightfoward approach relevant to your environment.

robtwister
Wednesday, October 15, 2003

I'm not sure what you mean by cleanup completely, but in java, I think thats what "Finally" is for.  Correct me if i'm wrong.

Vince
Wednesday, October 15, 2003

Joel's personal home page does include a Java applet game:

http://joel.spolsky.com/zoop/index.htm

Ged Byrne
Wednesday, October 15, 2003

"A plain old vanilla stack trace will tell you where the error occurred -- even in your original example. You can then "isolate and fix" it."

I think you're wrong, in general.  You're arguing for a _reactive_ approach to programming.  I much prefer to be proactive in catching errors, which means being able to easily see what, specifically, can go wrong in a given block of code.

At some point you've detached the "readability" issue from the "robustness" issue.  The latter follows from the former in programming environments, especially when a large team shares the same codebase.

As to the "runtime exception" issue that T Norman brought up, I would argue for a limited use--ie, as a function-local "callback" for the "index out of bounds" type nuisances.  I don't agree with the concept of a "higher level" handler, because then it becomes very difficult to clean up resources, when the thread can exit at practically any arbitrary point.

But in that capacity, it's not the try/catch mechanism per se that's meaningful, just the notion of having _some_ kind of net to catch "low level" exceptions/errors.  IMO, this is only a better-than-nothing kludge, not a voucher for the beauty of exception handling.  And I frankly refuse to take the use any farther.

smkr45
Wednesday, October 15, 2003

You can have a finally block without having to actually catch and handle any any exceptions, if you need to cleanup resources at a low level.

I am specifically referring to not catching RuntimeExceptions.  If those occur, it is because of a programming error or bad configuration data (e.g. messed up properties files).  Code shouldn't be written in low-level places to catch them; code should be written so they don't happen in production.  The high level handler is just a safety net for when your testing and code reviews failed to prevent the error in production.

T. Norman
Wednesday, October 15, 2003

>> "A plain old vanilla stack trace will tell you where the error occurred -- even in your original example. You can then "isolate and fix" it."

> I think you're wrong, in general.  You're arguing for a _reactive_ approach to programming.  I much prefer to be proactive in catching errors, which means being able to easily see what, specifically, can go wrong in a given block of code.

And I think you're talking through both sides of your hat.

I already showed you how *I* would design it (or refactor it), and in that design it's perfectly easy to see where problems occured.  I'd rather be proactive, to use your terminology, and do it that way.

But *you* said, "No it's legacy code, I can't just rewrite everything". So now you're in reactive approach, no? And, *given that approach*, stack traces work just fine.

> And I frankly refuse to take the use any farther.

In other words, "I've made up my mind and I'm not budging"

Portabella
Thursday, October 16, 2003

*  Recent Topics

*  Fog Creek Home