- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
And what is the problem with it? Basically, this function converts from 'success-or-exception' to 'success-or-null'. You say the caller has to check for null. That's true. In the other case, it would have to handle the exception. One of the two has to be done.
Admin
NevemTeve: Not really. With a null check it must be explicit at the point of return from getProject() or else the context will be lost when a NullReferenceException eventually occurs at the point of first use.
The point of the whole try/catch model is to make "fail fast" easy and unobtrusive; at the point of exception you do things like logging or maybe exception wrapping, then you let it bubble out to the top and crash the whole app. Or in the case of a server-ish app, abort & reset that particular task / connection / transaction / thread. But up near the top of that object / call stack, not down near at the point of failure.
In the frist model you're stuck littering your code with an if (somehowDetermineAllErrorStates) { } handler after almost every method call or property reference. In the second model you're not.
Seems a fairly clear choice to me.
Admin
The only possible exception (unless they have nuclear station launch code in conductor) is an out of memory. I'm wondering how much memory logging the exception consumes? It'd be real sad if it's more than creating an object.
Addendum 2019-03-07 07:16: Constructor*
Admin
Depends on how much code that constructor executes. It could easily do all sorts of complicated stuff, connect to databases, etc. And might throw an exception if any of those things fails.
I agree that throwing an exception out of a parameterless public (AKA default) constructor is very bad form. But for enterprisy hairy objects with names like "Project" not all that rare.
Part of the fun of TDWTF is guessing about the rest of the code we don't get to see.
Admin
Why is that the only possible exception? The constructor could have anything in it. Like a call to some kind of persistence layer or to append something to a logfile. These kinds of situations can throw "any" kind of exception.
Admin
The problem is that it's either useless or making things worse.
If you don't do anything about the exception, if will simply bubble up the call stack to whoever cares and it's done. If you return null, then the caller is forced to do something similar therefore it will simulate the exception behavior with more effort. If the caller will not do anything then it will fall back to previous behavior by NPE. It's clear, that at the end the result is the same and the extra handling code is not needed at all.
Admin
Yes, one or the other must be done. The beauty of this implementation is you get to do both. We could continue this pattern all the way up the stack:
If (project == null) throw new ProjectMissingException() ... catch (ProjectMissingException) { project = null } ... if (project == null) throw new HigherLevelProjectMissingException()
Admin
I agree. The pattern I like to follow is:
Boolean Function1 (out Result, ref Stack) { try { Result = somethingelse(); return (Result != null); } catch { Result = null; Add(Stack,Exception); Log(Exception); return (Result != null); } } Boolean Function2 (Stack) { try { Result = null; if (Function1(out Result, Stack) == false || Result == null) { throw new Exception("Call to Function1() failed."); } return true; } catch { Add(Stack,Exception); Log(Exception); return false; } }
Once you get into a pattern of checking for A) false returns and B) null objects coming back, it's really easy to write a very reliable and easy to debug system. If you name your stuff right and the bottom layer has semi-useful exception messages, sometimes the users can figure out what went wrong without your help (very helpful when it's more of a usage error than a bug). But some developers don't want to use exceptions consistently, or log useful details, or separate their code into logical pieces, or write decent error messages, so everything looks like "too much work." TRWTF is this snipped being submitted and accepted as WTF.
Admin
The biggest thing that offends me here is that the original exception is lost. Something went wrong, good luck figuring out what!
I'd say the exact opposite of what Tina's co-worker understood: Never catch exceptions that you don't understand. The fact that an exception occurred that you didn't understand means that you have a bug and you have no idea what the implications are. Best to gather diagnostic information and get back to a known state ASAP.
Admin
To all the complete and utter wastes of oxygen which are the majority of the correspondents on this site ...
Come on, at the very least log the exception message.
Admin
The problem is that they aren't actually handling the exception. Just logging and kicking the can. The general rule of thumb in java is to ask yourself "can I actually handle this exception in the given context?" If the answer is no you throw the exception. You don't catch log and return null.
Admin
Yes, it's just an impedance mismatch between two layers of code (perhaps the lower layer is a third party library with no source available). Translate and log. Nothing wrong with that. I've had a number of occasions when I've written a "catch" which does nothing but write a log message - you need to know what the logging, monitoring and alerting systems are doing with the log entry to have a full understanding of this code.
Admin
A try catch is not always the proper way to handle an error, in this case it might be assumed that the caller will have some kind of error handling that wouldn't be appropriate in a catch block. Without knowing the context of how this is being called the only real issue here is not logging more details to the log.
Admin
While it certainly stinks of code rot, I do not think it is an [completely] illegitimate means of handling. It primarily just convert the means of handling and assumes that regardless of type of failure continuation is impossible. If I had to critique one thing, it would be that the exception itself is not logged. The only reason I can think of doing so is that there is exactly one reason why initialization would have failed. It is, however, for this reason why I usually try to avoid any operation that could result in an exception in a constructor if it makes sense in the design. In this case, I would feel more comfortable if Project was an interface (IProject for the dot-net folks) and the constructed class was "new ProjectImplemented();"
Admin
No. The real issue is catching the exception at all. As Dank mentioned (and it applies to .Net as well), don't catch an exception unless you can either handle it gracefully or add some valuable information.
Deal with the logging somewhere else. i.e. a Global Exception handler. Or use a nice library like log4net.
Admin
In java you have checked exceptions. Anything that isn't a checked exception is a runtime exception. Runtime exceptions are usually caused by a logical error vs checked exceptions which are considered unavoidable(i.e. connection timeout). Runtime exceptions should be fixed at their source rather than being caught. The compiler and your IDE force you to address checked exceptions by either catching them or throwing. In short: They didn't because the compiler didn't force them to.
Admin
Finally, someone who understands the benefit of exceptions over return codes.
Admin
Aside from the ridiculous amount of boiler-plate code a policy of catching and logging exceptions inevitably leads to a lot of repeated messages in your log file... all the way up the call stack.
Admin
There is probably some interesting stuff further up the call stack where it catches any null reference exception and retries, but lets every other exception die.
Admin
Presumably this is from an enviroment that can give you the call stack, right?
Because I've seen something like that done in environments that couldn't give you the call stack: you got log messages at the point of error and on up, but the actual error was handled by code at the appropriate level (often, the user interface level)
Admin
Cut out the middle man:
throw (Exception) null;
;)
Admin
Don't know about this one. It actually seems to be sort of internal method for potentially getting Project object while not really caring about possible problems with instantiation. Might be WTF or the OP might just have gotten confused by the availability of getProject() method which was not intended for the purpose he was trying to use it for. Not really possible to tell without context which is not provided here.
Admin
The point of exceptions is they make the application fail such that you read a simple stack trace and see a chain of events leading up to it and, very often, fix it just based on an auto-generated stack trace. It's not a good model in that when you need certain code to be robust, it's a trial and error process to accomplish this, but it can be done.
If you've dug through log traces or stepped through code, you know why nulls are so painful: Nulls hide the point at which the error occurred, and discard any information about the error. That null could get passed through other pieces of code, written to a file and days, months, however later you find, "oh, I've got these unexpected nulls in this file, where the hell did that happen?"
This is really a hidden complexity issue. If you let things fail and fix them, your code handles actual data. You can express your inputs and outputs in plain English, and your function is called with one possible set of N arguments. When you return nulls, now every piece of data means "a thing" or "this is missing for some unknown reason" and now N arguments becomes 2^N possible states, most of which just indicate brokenness. So you have to add more defensive code, more complexity that does nothing useful, to put off fixing something you're probably going to need to fix anyway.
Admin
Sometimes you don't care WHY you didn't get back the [Project]. If it's there you can return it, if it isn't there or there was an error with getting it you return a null.
If there is a problem with getting the [Project] there the calling code probably can't do anything about it. An obvious example of this is UI code. Maybe there displaying lots of different things. You still want to display the page even if there is no [Project].
To put it simply, I don't think that this is a WTF.