- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- 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
7/10 to TDWTF for posting this.
Pro tip: If you are a FNG and think you found a whole load of WTF, you probably haven't.
Admin
To be fair, an ArgumentException containing the name of the argument is a lot more helpful than a NullReferenceException if it just gets caught somewhere and dumped to an error log.
Admin
"Here's a nickel, kid. Go buy yourself a real type system."
The compiler won't let you pass the results of findInvestorBySsn directly to investorsAccountInfo because the types are different.Admin
The code analysis tool built in to MS Team Foundation Server thinks the opposite and warns if a parameter is used before ensuring it is not null.
Also, the description of NullRerefenceException in MSDN is "The exception that is thrown when there is an attempt to dereference a null object reference." Following this, a NullReferenceException should only be thrown if an object is explicitly null at the actual de-reference, not during some sort of sanity check.
Throwing an ArgumentExpception ("The exception that is thrown when one of the arguments provided to a method is not valid."), ArgumentNullException ("The exception that is thrown when a null reference (Nothing in Visual Basic) is passed to a method that does not accept it as a valid argument.") or ArgumentOutOfRangeException ("The exception that is thrown when the value of an argument is outside the allowable range of values as defined by the invoked method.") would be exactly using them as intended! In fact, the author of this "WTF" is using them exactly as MSDN suggests!
Also, public Microsoft .Net framework and SharePoint libraries use this kind of guarding extremely frequently. (Although not including the assertion). Some might think that that is a bad idea but then we might just as well put "OMG Midas works at a company that appearantly uses Windows applications frequently!" as tomorrows Daily WTF...
Regarding the "trained chimp using your code" - no, that's not the reason for the check. Anyone, including you, can accidentally pass a null reference to some method, and the check is meant to make it easier to find the source of the problem. Sure, calling SomeMethod(null) would be stupid, but a null reference may of course be trigged by some bug in a very complex piece of code or may even not be the user's fault (perhaps another method in your own library has a bug and accidentally returned a null reference).
Admin
A popular one I see in our old code is if (booleanCondition == true) {} else { //real code }
Yes, it's written on one line like that so at first glance you don't notice the "{} else"
Admin
TL;DR
Argument about NullReferenceException vs ArgumentException:
Even in C you check arguments for valid values and return an error code if they are invalid. And then check those codes, show error messages, and continue work. Which is far more preferable to a segmentation fault resulting from a bad null pointer.
The difference from .NET (and Java, and probably a lot of other languages) is that .NET prefers exceptions to error codes. This is the same argument checking, just you throw an exception instead of returning an error code.
Of course, a NullReferenceException would also be thrown, but the arguments against it have already been mentioned in the first page - unpredictable program state and less clarity of what actually went wrong. The same principle applies in C - the sooner you catch an error condition, the smaller the mess.
Of course, .NET has an ArgumentNullException, which would be more appropriate here, but that's a minor flaw. As is the "== false" thing. Not exactly the best practice, but nothing to cry over either.
Admin
Admin
This.
You write code. You test it. You get a NullPointerException. You look at the stack trace. You fix it. Problem solved.
The code in the article simply substitutes one exception for another, so it doesn't add any real value.
As always TRWTF is in the comments. Specifically everyone who said this is not a wtf.
Causa I said so.
Admin
Since Sense Mute Moot?
Admin
It's not the same as checking for non-nullness, and I'm not sure if a PMD check exists for it. It's not something you usually do in Java anyway.
The combination of warnings/errors by the Java compiler, Netbeans and the PMD plug-in gives an awful lot of functionality when it comes to source code checking, by the way. Your code quality will increase greatly.
Anyway, what I was talking about is that PMD complains about things like '== true' or '== false', or 'if( expr ) return true; else return false;' instead of just 'return expr;', or 'if( !expr ) method1(); else method2();', and all that sort of thing.
In Java, it's a bit different: So really and truly, we're talking apples and oranges again. In Java, it's perfectly permissible to throw a NullPointerException, except that it's a code smell for most applications. The standard libraries use it liberally, though. Absolutely, and it happens all the time. But in such cases, you can use the assert keyword. It's been in Java since 1.4, and that's from early 2002.I have to admit that I never use it myself, but perhaps this is something I ought to consider. After all, they can be switched off during runtime, so there's no or hardly any performance impact.
And since I'm the one responsible for every developer sticking to a set of requirements for source code (variable names, how to use exceptions, use of PMD, etc.) I can make it compulsory for everybody. :)
Admin
We used to have a scripting language where IF could be followed by any number of lines prefixed with THEN, ELSE, or nothing, and they would execute based on the last executed IF. So you could say
So, if "cond" were true, you'd execute statements 1,2,3,4,6,7; if "cond" were false, you'd execute statements 0,1,4,5,7,8. Of course, you'd be crazy to make use of this feature, but you could. For some reason, that wasn't adopted by later languages. Go figure.
Admin
Or, the error is very rare. Happens in production only. No way to debug. Maybe the method that throws has three or four nullable parameters, which are references assigned several methods up the stack trace. Have fun finding which one is null and when and why...
The ArgumentNullException occurs at least a little closer to the source, and (if properly used, with the "argument" property set on the exception you know WHICH object is null, and when it is so.
Admin
"The docs say pass non-null, but I'm going to pass null anyway because I know what I'm doing I'm a genius." Then they compile. "WTF? Null-who? This library sucks ass, time to rewrite our codebase to use that new rockstar API - I heard it has a guard for those null thingers."
Admin
Admin
Visual Studio as well, does not compile when a variable is not initialized, however, doing SomeClass someObject = null; is perfectly legal AND MUST of course be perfectly legal and there are tons of framework libraries that returns null in many conditions.
However, we were talking about your PMD complaining about ArgumentNullException being a code smell, which Microsoft doesn't.
Appearantly the "policy" for NullReferenceException is different in Java but I must say the Java way here seems kind of retarded.
I don't see what the benefit of disabling this kind of assertions in runtime would give? I mean, basically, deciding wether to have this test is basically "less code vs easier debugging". Having an assertion and disabling at runtime gives "more code" bot no "easier debugging".
Admin
A good programmer is someone who always looks both ways before crossing a one-way street.
Admin
Admin
Well thank God he caught the null reference and enum not defined exceptions! After all, there's no way those would be inevitably caught in the body of the method...
Admin
Admin
That's like giving your car's bumper its own bumper. It's a WTF.
If it's actually going to cause a problem if you let a null slip through, then of course you check it. But this is almost never the case; 99.0% of the time, you don't need all that redundant bullcrap.
Admin
Admin
Should we double check to make sure no one did these incredibly stupid things?
Not that I'm disagreeing with argument validation.
Admin
The problem is, though, that a NulllPointerException indicates a bug. If you're still accessing null objects in production code, you have an issue with your testing. This point has been put several times now, by a variety of posters, but it doesn't appear to hit home.
Admin
I inherited a codebase that had this beauty all over the place:
switch(someboolean) { case true: blabla; break; }
Sometimes there was a 'case false' as well...
btw, we're talking C# here so the boolean is a real boolean. Without a 'maybe'....
Admin
Yeah. It's like driving; 99.0% of the time you wont get into an accident.
Who needs seat belts and airbags?
Admin
Do you have loosely coupled application and library code?
Does any of your code interoperate with other systems?
These are some examples of situations where this type of validation is important, and they not analogous to your explicitly stupid/malicious examples.
Admin
I'm just going to leave this here:
https://commons.apache.org/lang/api-2.6/org/apache/commons/lang/Validate.html
Admin
There's been a lot of talk about whether it is proper or not to check the input of your function at the forefront or to wait until it causes an error because of the (implicit) assumptions you have made. Allow me to offer a mathematics perspective.
Generally, you can say that a procedure is true if the input parameters are within a certain bounds. Consider, for example, a function that accepts a probability. What type do you use as a parameter? double? For a double x, 0 < x < 1 is nonsense, and yet a C-syntax programming language does not allow you to specify this restriction. What else can you do but throw an (Java) IllegalArgumentException? How about square roots? C may have an unsigned type (and I can't speak for C-pound), but Java does not. There IS a valid result for a negative number, but that is not accurately expressed using any of the primitive types.
Finally, I will submit this example, and it is my best one. What happens if you divide a positive number by 0? Any mathematician will tell you that the answer is not determined. However, in Java, the answer will be Number.POSITIVE_INFINITY. Obviously, this is true for limits, but not for an ACTUAL CALCULATION. Any function that accepts a divisor should therefore check it for zero and throw a IllegalArgumentException if it is expected to be non-zero.
In conclusion, a function will be most robust if its limits are clearly defined. In C-based languages, this is not possible to do by declaration. By defining the boundaries using exceptions, the code (in particular, public APIs) can greatly benefit in terms of stability and can avoid strange and undesired side-effects.
Admin
hoodie's 99% was intended to allow an exception (no pun intended) to the rule - the remaining 1% provided for the potential (though doubtful) situation where it might be better to catch a null exception. He wasn't excusing himself from safety measures just because the risk is low, as you suggest with your silly comparison.
Admin
Every company hires chimps... if your company doesn't seem to, it just means you're the chimp.
Admin
The [AssertionMethod] Attribute means that the function is removed from release code (and the evauluation of the arguments). This is similar to a preprocessor assert in C/C++ code, with many of the same benefits/drawbacks. In release code you will get your NPE, and get your added performance of not having to worry about trained chimps, while in debug it fails early with a descriptive exception with a helpful text explanation.
Admin
Who needs a bumper on their bumper?
...yo dawg...
...sorry, couldn't help it.
Admin
Admin
Does it really do that?
Mathematically thats a bit odd, as division by zero should either be not defined (and throw some kind of error, be it compiletime or runtime), or specified as some kind of NaN value. (Yes I know the IEEE have some kind of lame standard).
Admin
Admin
Admin
Yes... clearly it ought to be
:o)
Admin
Admin
To me, documenting what a) returns null, and b) accepts null is how you handle this on the development end. You'll have a stack trace in development and, without compiler optimizations on, it will lead you straight back to the source of the problem, usually in seconds.
Some systems are too freakishly complicated for that though, in which case they should probably be simplified, with parameter checking as a stop-gap measure until that can be done.
Admin
"If I have 2 oranges and give them both away, I suddenly have nothing. . ."
I think the better analogy would be if you went to the grocery store and asked the produce guy for 2 oranges. He goes away, and comes back empty handed. Was it because they were out of oranges, or was it because he only speaks Polish and didn't understand what you said? Or maybe he doesn't have any hands? Or you're actually at a shoe store?
Admin
What you're describing sounds more like the application side of things to me. Library code (like the OP's) is generally not in the business of error handling. The choice for the library coder is where to validate input (or not all). Parameters are a natural choice because they are part of an interface. It's a seperation of concerns thing.
Admin
Admin
int i = 42; bool b = ((bool)&i); switch(b) { ... }
Admin
so many tldr comments and tldr replies. no one cares, moby! no one!
Admin
Admin
So I take it that != and ! are banned? So that these are "intolerable"?
if (!booleanCondition) { //real code }
Or if you must:
if (booleanCondition != true) { //real code }
(Shudder.)
The only thing I can think of that would be worse is if they buried the else in a define of some type:
if (booleanCondition == true) IC { //real code }
Admin
No it doesn't do that, you'll get an ArithmeticException.
Admin
If I were to write a library that gets sold to the public outside my organization, I would validate every input on public methods/ctors. But in my case, which is usually internal library and both internal and external app dev, the intellisense is enough.
Addendum (2011-08-30 14:28): Okay, maybe I validate more than 1% of the time. Maybe up to 5%.
Just looking at what's on my left two screens right now, I am throwing ArgumentNullException, InvalidOperationException, and EndOfStreamException - all validating some form of input. The ArgumentNullException and InvalidOperationExceptions are there so that people who derive from a certain one of my abstract classes can know for certain when they are doing it wrong; I like to give extra help to implementers of my abstract base classes, since inheritance is not as intuitive as composition.
The EndOfStreamException is more for communications reasons; my code can detect when the stream ends before it should have since the stream contains XML.
Admin
captcha: delenit
Admin
Oops, I stand corrected!