Most languages these days have some variation of “is string null or empty” as a convenience function. Certainly, C#, the language we’re looking at today does. Let’s look at a few example of how this can go wrong, from different developers.
We start with an example from Jason, which is useless, but not a true WTF:
<span class="hljs-comment"><span class="hljs-doctag">///</span> <span class="hljs-doctag"><summary></span></span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> Does the given string contain any characters?</span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> <span class="hljs-doctag"></summary></span></span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> <span class="hljs-doctag"><param name="strToCheck"></span>String to check<span class="hljs-doctag"></param></span></span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> <span class="hljs-doctag"><returns></span></span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> true - String contains some characters.</span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> false - String is null or empty.</span>
<span class="hljs-comment"><span class="hljs-doctag">///</span> <span class="hljs-doctag"></returns></span></span>
<span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-keyword">static</span> <span class="hljs-built_in">bool</span> <span class="hljs-title">StringValid</span>(<span class="hljs-params"><span class="hljs-built_in">string</span> strToCheck</span>)</span>
{
<span class="hljs-keyword">if</span> ((strToCheck == <span class="hljs-literal">null</span>) ||
(strToCheck == <span class="hljs-built_in">string</span>.Empty))
<span class="hljs-keyword">return</span> <span class="hljs-literal">false</span>;
<span class="hljs-keyword">return</span> <span class="hljs-literal">true</span>;
}
Obviously, a better solution here would be to simply return the boolean expression instead of using a conditional, but equally obvious, the even better solution would be to use the built-in. But as implementations go, this doesn’t completely lose the plot. It’s bad, it shouldn’t exist, but it’s barely a WTF. How can we make this worse?
Well, Derek sends us an example line, which is scattered through the codebase.
<span class="hljs-keyword">if</span> (Port==<span class="hljs-literal">null</span> || <span class="hljs-string">""</span>.Equals(Port)) { <span class="hljs-comment">/* do stuff */</span>}
Yes, it’s frequently done as a one-liner, like this, with the do stuff jammed all together. And yes, the variable is frequently different- it’s likely the developer responsible saved this bit of code as a snippet so they could easily drop it in anywhere. And they dropped it in everywhere. Any place a string got touched in the code, this pattern reared its head.
I especially like the "".Equals call, which is certainly valid, but inverted from how most people would think about doing the check. It echos Python’s string join function, which is invoked on the join character (and not the string being joined), which makes me wonder if that’s where this developer started out?
I’ll never know.
Finally, let’s poke at one from Malfist. We jump over to Java for this one. Malfist saw a function called checkNull and foolishly assumed that it returned a boolean if a string was null.
<span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-keyword">static</span> final String <span class="hljs-title">checkNull</span>(<span class="hljs-params">String str, String defaultStr</span>)</span>
{
<span class="hljs-keyword">if</span> (str == <span class="hljs-literal">null</span>)
<span class="hljs-keyword">return</span> defaultStr ;
<span class="hljs-keyword">else</span>
<span class="hljs-keyword">return</span> str.trim() ;
}
No, it’s not actually a check. It’s a coalesce function. Okay, misleading names aside, what is wrong with it? Well, for my money, the fact that the non-null input string gets trimmed, but the default string does not. With the bonus points that this does nothing to verify that the default string isn’t null, which means this could easily still propagate null reference exceptions in unexpected places.
I’ve said it before, and I’ll say it again: strings were a mistake. We should just abolish them. No more text, everybody, we’re done.
[Advertisement]Source: Read MoreÂ