Comment On There's More Than One Way to Null a String

I'm guessing you changed the variable names because that would appear to always return false and ret isn't defined in the method.  If ret is an instance variable... well, I don't even want to think about that.

I hope that "ret=true" really means "isnull=true", otherwise all that beatuiful short-circuit evaluation is for nothing...

The best part is that it'll always return false. :P

I sense the future will soon bring Issue #781335:



str.equals("_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null_null")

Miszou:

I hope that "ret=true" really means "isnull=true", otherwise all that beatuiful short-circuit evaluation is for nothing...

Of course the whole thing could be:

return (str == null) ||
         (str.equals("__")) ||
         (str.equals("__null")) ||
         (str.equals("_null_")) ||
         //Added for Issue #107724
         (str.equals("_null_null")) ||
         (str.equals("null__")) ||
         (str.equals("null__null")) ||
         //Added for Issue #126185
         (str.equals("null_null_")) ||
         (str.equals("null_null_null"));

And I hate all the extranneous parens.  Why do poeple think that makes it more readable?

return (str == null
     || str.equals("__")
     || str.equals("__null")
     || str.equals("_null_")
     || str.equals("_null_null") //Added for Issue #107724
     || str.equals("null__")
     || str.equals("null__null")
     || str.equals("null_null_") //Added for Issue #126185
     || str.equals("null_null_null"));

is much better IMO.  Or even better, create this during class loading:

Set nullStrings = new HashSet();

nullStrings.add("__");
nullStrings.add("__null");
nullStrings.add("_null_");
nullStrings.add("_null_null"); //Added for Issue #107724
nullStrings.add("null__");
nullStrings.add("null__null");
nullStrings.add("null_null_"); //Added for Issue #126185
nullStrings.add("null_null_null");

Then the method becomes:

return str == null || nullStrings.contains(str);

In terms of the actual WTF here, it's hard to say whether this is the result of bad code in the application or a third party tool.  I've had to do stuff like this to deal with both.  So this looks like a work around to a WTF to me, not the WTF itself.

I'm not entirely sure that I should read this blog. I have nasty
flashbacks to the system my cousin and I helped to maintain (and
untangle); a bookkeeping program written in Pascal and which had about
4000 global variables and not one single local variable.



That was fun. Not.

Is anyone else surprise that there are at least #126185 issues reported so far?

<sarcasm>

What about the value of NULL or _NULL or the infamous _NULL_NULL?  I think the code is buggy and needs to take those risks into consideration.

</sarcasm>

Someone's data is in need of a good cleaning!

Bustaz Kool:

Is anyone else surprise that there are at least #126185 issues reported so far?

And how did you come to that conclusion?

dubwai:

So this looks like a work around to a WTF to me, not the WTF itself.

I agree. 

I think the submitter of this code should tell us a little about the project and why something like this is necessary.

Miszou:

I hope that "ret=true" really means "isnull=true", otherwise all that beatuiful short-circuit evaluation is for nothing...

It does. Ugh, I *really* need to get better at proofing the "anonymizing" a bet better ... thanks for pointing it out.

RyGuy:

dubwai:

So this looks like a work around to a WTF to me, not the WTF itself.

I agree. 

I think the submitter of this code should tell us a little about the project and why something like this is necessary.

I agree as well.

It looks like the author is trying to handle what the system "thinks" is null in a nice way...this system seems to have multiple ways of setting variables to 'null'.

Alex Papadimoulis:

Miszou:

I hope that "ret=true" really means "isnull=true", otherwise all that beatuiful short-circuit evaluation is for nothing...

It does. Ugh, I *really* need to get better at proofing the "anonymizing" a bet better ... thanks for pointing it out.

How about trying to compile the snippet after you modify it?

Alex Papadimoulis:

Miszou:

I hope that "ret=true" really means "isnull=true", otherwise all that beatuiful short-circuit evaluation is for nothing...

It does. Ugh, I *really* need to get better at proofing the "anonymizing" a bet better ... thanks for pointing it out.

Hey Alex, it would be a good idea to keep the original post, with mistakes crossed out and  new lines  added.  It is not funny when you see people are talking about ret=true and you can't find it in the post.

Hey at least it's commented!

Maybe a regular expression that would match the word "null" with any number of leading, trailing or in-between underscores would be nicer... oh, wait! This wouldn't match "__". Darn it!

Sweet sassy molassy. If I saw this in production code in my office, I
think I'd quit immediately and maybe even set fire to the building on
the way out.



Interestingly, an empty string will return false. Not sure if that's an oversight or an f'ed up twist on the rule.

Evidently, "null" is not null.

Makes sense.

I've had to write code similar to this. A third party (stock market
data provider) started sending us strings containing the word "null"
instead of just empty strings. Without telling us first.

dubwai:

And I hate all the extranneous parens.  Why do poeple think that makes it more readable?

(((why worry about ((operator precedence?))))

John Bigboote:

Sweet sassy molassy. If I saw this in production code in my office, I
think I'd quit immediately and maybe even set fire to the building on
the way out.



Interestingly, an empty string will return false. Not sure if that's an oversight or an f'ed up twist on the rule.

Without knowing the context in which this function was used I can't share your sentiments.

I agree with the people who say this is actually an attempt to clean up other wtf code.



The thing which interests me is that, among all the things they allow as a null string, an empty string is not one of them.



Given that it appears that bad code elsewhere in the system
concatenates "null" strings and then other code that gets these
concatenated null strings works better if the two concatenated strings
are considered null, it follows that it is happy to accept a null
string concatenated with an empty string is null ... so why not empty
strings?



Indeed this looks like a bunch of extremely bad code written to avoid
the apparently dire consequences of assuming empty strings are null. No
doubt one can have fun with their system by entering the surname "__"
or "_null".

WTF. Why not just use str.Find("null")
and str.Find("__").  That would handle all of these stupid cases,
including Issue #eventy-billion.  That is, unless for some damn
reason "null" is a valid string.

        //Added for Issue #107724
         //Added for Issue #126185

now THAT is priceles. What is exactly issue number X? Reading too much slashdot on the job?

Any developer knows it should be

public static boolean IsNull(String str)
{
      return (
         (str == null) ||
         (str.equals("__")) ||
         (str.equals("__null")) ||
         (str.equals("_null_")) ||
         //Added for Issue #107724
         (str.equals("_null_null")) ||
         (str.equals("null__")) ||
         (str.equals("null__null")) ||
         //Added for Issue #126185
         (str.equals("null_null_")) ||
         (str.equals("null_null_null"))
       );
}

right? ;)

dubwai:

Miszou:

I hope that "ret=true" really means "isnull=true", otherwise all that beatuiful short-circuit evaluation is for nothing...

Set nullStrings = new HashSet();

nullStrings.add("__");
nullStrings.add("__null");
nullStrings.add("_null_");
nullStrings.add("_null_null"); //Added for Issue #107724
nullStrings.add("null__");
nullStrings.add("null__null");
nullStrings.add("null_null_"); //Added for Issue #126185
nullStrings.add("null_null_null");

Then the method becomes:

return str == null || nullStrings.contains(str);

If this is Java (which I suspect from the return type of boolean rather than bool), then you can also do nullStrings.add(null) and nullStrings.contains(null) will return true.

I've often wondered about the following common paradigm:

boolean result = false;
if (blah || bleah || yadda || bubba) then result = true;
return result;

I keep thinking: why not return blah || bleah || yadda || bubba...

I suppose life could be worse...

return (blah || bleah || yadda || bubba) ? true : false

I don't normally defend WTF posts, but I have to agree with some of the others posting so far. On more than one occaision, I've had to write one-time tools like this to either fix a user mistake where thousands of database records were changed, or to fix third party software errors where another tool freaked out and changed data. Then again...

Hardcoding each of the potential possible values for null is just retarded. As soon as someone comes across "_null__" or "__null_null" or some other bizarre combination of "null" and underscores, the tool will have to be recompiled. Use regular expressions for god's sake!!!

Why would anyone want to use null_null_null though?

http://www.google.tt/search?q=null_null_null&sourceid=mozilla-search&start=0&start=0&ie=utf-8&oe=utf-8&client=firefox-a&rls=org.mozilla:en-US:official

The real WTF is what kind of a mega-super-moronic system *created* those various assignments that must all be treated as null strings?

I mean... "null__null" as the *value* of a string to mean that the string is null?

What the FFFFFFFFFFFFFF?

Whatever happened to ""?

John Smallberries:

dubwai:

And I hate all the extranneous parens.  Why do poeple think that makes it more readable?

(((why worry about ((operator precedence?))))

True, str.equals("blah") || str.equals("blah") really makes me scratch my head trying to figure out the operator precendence.  You'd think it would evaluate to the result of the first method call or'd with the result of the second method call.  But you never know, it could also evaluate to the result of the first method call or'd with the result of the second method call or even the result of the first method call or'd with the result of the second method call.  It's really hard to say.

Now (str.equals("blah")) || (str.equals("blah")) will definitely evaluate to the result of the first method call or'd with the result of the second method call.

Totally.

Gawd... I think I know where __null and null__ and all other variants comes from.

It looks like a string is used to hold values for several fields at once, like a comma-separated line, except using __ as the delimiter, or like a bitmask.

So if the semantics of the strings is

lastname__firstname__zipcode

you could have

papadimoulis__alex__12345

but if Alex was a naughty boy he may have left out his last name or zip code or what have you and different versions of the interface may have assigned "" or "null" to represent null fields, thus giving rise to

__alex__

__alex

__12345

etc.

I hope I'm wrong.

rogthefrog:

Gawd... I think I know where __null and null__ and all other variants comes from.

It looks like a string is used to hold values for several fields at
once, like a comma-separated line, except using __ as the delimiter, or
like a bitmask.

So if the semantics of the strings is

lastname__firstname__zipcode

you could have

papadimoulis__alex__12345

This would explain why no test for "" or "null" since the input would always have the underscores added before the test.

Reloacate to Cleveland! +o(   Now there is a WTF!



Sorry, I lived in the "mid-west" once before.  I aint going back!

I'll stay in the Rockies.

dubwai:

John Smallberries:

dubwai:

And I hate all the extranneous parens.  Why do poeple think that makes it more readable?

(((why worry about ((operator precedence?))))

True, str.equals("blah") || str.equals("blah") really makes me scratch my head trying to figure out the operator precendence.  You'd think it would evaluate to the result of the first method call or'd with the result of the second method call.  But you never know, it could also evaluate to the result of the first method call or'd with the result of the second method call or even the result of the first method call or'd with the result of the second method call.  It's really hard to say.

Now (str.equals("blah")) || (str.equals("blah")) will definitely evaluate to the result of the first method call or'd with the result of the second method call.

Totally.

LMAO! - [:D]

Anonymous:

I've often wondered about the following common paradigm:

boolean result = false;
if (blah || bleah || yadda || bubba) then result = true;
return result;

I keep thinking: why not return blah || bleah || yadda || bubba...

Depends on the function.  I use the paradigm to avoid multiple
exit points.  Instead of returning from some location within the
if () {} else {} statement, I set the return value and have only one
point where the functino returns.  If it's a simple boolean
return, or something directly computed, that's a different story. 
It's a matter of knowing when to use the paradigm and knowing when not
to use it.

About the job position, if my code was posted on your site, does that count as a refecence ? ;-)

Anonymous:

Reloacate to Cleveland! +o(   Now there is a WTF!

In addition to rocking (you've heard the song, right?), we are officially #1*.

* poorest city in the US

rogthefrog:

The real WTF is what kind of a mega-super-moronic system *created* those various assignments that must all be treated as null strings?

I mean... "null__null" as the *value* of a string to mean that the string is null?

What the FFFFFFFFFFFFFF?

Whatever happened to ""?

"" != null in Java.

Rick:

rogthefrog:

Gawd... I think I know where __null and null__ and all other variants comes from.

It looks like a string is used to hold values for several fields at once, like a comma-separated line, except using __ as the delimiter, or like a bitmask.

So if the semantics of the strings is

lastname__firstname__zipcode

you could have

papadimoulis__alex__12345

This would explain why no test for "" or "null" since the input would always have the underscores added before the test.

Correction to my post: the separator is _ not __. The occurrence of __ signifies that all 3 possible values in the string are blank.

If this is right (which it probably is, sadly) this design is pretty moronic.

There are, however, possible explanations. Say the app is tracking tasks, each of which returns some string (which may be null). You could store each task's result in some session variable, and when your user is done write it to your db as a single string separated by _. This makes it extensible: if you add another task, you don't have to add a field to your db. If each task's result uniquely identifies the task, this schema allows you to data-mine your db for questions like "how many users did task X right before task Y and didn't return null for either task"; "how many users stopped the whole process immediately after task X"; etc. You could store one record per task for each user but that would mean a lot more rows in your db and make said data mining possibly harder.

Agreeing with everyone elses comments about this guy, it's not that
function, but the rest of the code that would seem to be the WTF. Of
course, that's also what Alex said in his post about not even wanting
to see the rest of it.



But on a side note, I do enjoy this one:

Alex Papadimoulis:

so if you've got experience in your line of work

I'd have to argue that all people have experience in their own line of work... :P

Anonymous:

I suppose life could be worse...

return (blah || bleah || yadda || bubba) ? true : false

This can be useful under certain circumstances to avoid C4800 compiler warning.

Your choice:

return (blah || bleah || yadda || bubba ) ? true : false

return (blah || bleah || yadda || bubba) != 0;

 

Anonymous:

Alex Papadimoulis:

so if you've got experience in your line of work

I'd have to argue that all people have experience in their own line of work... :P

Consider recent graduates looking for jobs. I was trying to dissaude those (unfortunate) folks ;-).

mizhi:

Depends on the function.  I use the paradigm to avoid multiple exit points.  Instead of returning from some location within the if () {} else {} statement, I set the return value and have only one point where the functino returns.  If it's a simple boolean return, or something directly computed, that's a different story.  It's a matter of knowing when to use the paradigm and knowing when not to use it.

I don't find the single exit point to make anything clearer.  It makes the code path merge back on itself artifically (making the code more complex) instead of a clean branch.  It keeps the compiler from ensuring that all the code paths actually return something meaningful.  I like to be able to follow the code and easily see what it returns.  I don't like to have to walk through a bunch of extra code to see whether the returnValue variable is changed later on in the method.

Alex Papadimoulis:

Anonymous:

Reloacate to Cleveland! +o(   Now there is a WTF!

In addition to rocking (you've heard the song, right?), we are officially #1*.

* poorest city in the US

Arguably the best symphony in the country and a fantastic antique car museum, though

skicow:

RyGuy:

dubwai:

So this looks like a work around to a WTF to me, not the WTF itself.

I agree. 

I think the submitter of this code should tell us a little about the project and why something like this is necessary.

I agree as well.

It looks like the author is trying to handle what the system
"thinks" is null in a nice way...this system seems to have multiple
ways of setting variables to 'null'.

I'd say it's a WTF on both ends.  You'd think the developer would
maybe, just maybe try to either determine the cause of the various
forms of strings-containing-nulls that come or at least try to do some
sort of simple data cleansing so he/she doesn't have to add a new
conditional every time a new issue comes in.

rogthefrog:

There are, however, possible explanations. Say the app is tracking tasks, each of which returns some string (which may be null). You could store each task's result in some session variable, and when your user is done write it to your db as a single string separated by _. This makes it extensible: if you add another task, you don't have to add a field to your db.

Storing delimited strings in database fields makes baby Jesus cry.

 

haveworld:

http://www.google.tt/search?q=null_null_null&sourceid=mozilla-search&start=0&start=0&ie=utf-8&oe=utf-8&client=firefox-a&rls=org.mozilla:en-US:official

Why doesn't it surprise me that Oracle is in the result set?

A Wizard A True Star:

rogthefrog:

There are, however, possible explanations. Say the app is tracking tasks, each of which returns some string (which may be null). You could store each task's result in some session variable, and when your user is done write it to your db as a single string separated by _. This makes it extensible: if you add another task, you don't have to add a field to your db.

Storing delimited strings in database fields makes baby Jesus cry.

Baby Jesus deserves it. Sometimes that's actually a good way to do things.