• Araqnid (unregistered)

    arghfl.

    Not to mention, StringUtils.isBlank() would work fine....

  • (cs)

    The first version of the method had a real bug. The method did not do what its name implied, but that's no WTF.

    The second version of the method is still no good class design, but it ain't no WTF either, because there are some scenarios where you get objects, you don't know what they are (although you suspect them to be strings...), and you simply want to test whether they happen to be empty strings. Deserialization and generic data structures come to mind.

    Of course, a more compact way of doing things would be:

    public static boolean isEmptyString( Object obj ) { return (obj instanceof String && ((String)obj).Equals( String.Empty)); }

    So where's the WTF?

  • (cs) in reply to hanno

    OK now I got it. Call it with an Integer and it returns true. Sometimes your mind wants to refuse to see how bad things are.

  • Winter (unregistered) in reply to Araqnid
    Araqnid:
    arghfl.

    Not to mention, StringUtils.isBlank() would work fine....

    From the comment it looks like he wants to return true if there's a string of whitespace so I'd say isEmpty is the one he wants. Also checking to see that the object is a String at all. Other than that I agree this is totally reinventing the wheel, but reinventing it with corners.

  • been there (unregistered)

    if ( obj instanceof String ) { // if it's a string if ( ((String)obj).length() == 0 ) // and == "" return false; // then NOT emptyString - WTF?! }

    captcha = whiskey - mmm - need some...

  • Winter (unregistered) in reply to Winter
    Winter:
    Araqnid:
    arghfl.

    Not to mention, StringUtils.isBlank() would work fine....

    From the comment it looks like he wants to return true if there's a string of whitespace so I'd say isEmpty is the one he wants. Also checking to see that the object is a String at all. Other than that I agree this is totally reinventing the wheel, but reinventing it with corners.

    Edit: I mean return that a string of whitespace isn't an empty string. So false not true.. I think...

    CAPTCHA is craptastic, appropriately

  • (cs) in reply to hanno

    To quote Shakespeare:

    It is code written by an idiot, full of conditional statements and casts, validating nothing.
  • (cs)

    This code is FUBAR!

    Check this out...

    public class Test {
        public static boolean isEmptyString( Object obj )
        {
            if ( obj == null )
                return false;
    
            if ( obj instanceof String )
                {
                    /* a bunch of spaces is not an empty string */
                    if ( ((String)obj).length() == 0 )
                        return false;
                }
    
            return true;
        }
    
        public static void main(String[] args) {
            Object[] tests = new Object[] {
                "Hello",
                "",
                "  ",
                "\u0021",
                new Integer(0)};
                
            for (Object o:tests) {
                System.out.println("Is '" + o + "' an empty string? " + isEmptyString(o));
            }
        }
    }
    

    This prints out:

    Is 'Hello' an empty string? true
    Is '' an empty string? false
    Is '  ' an empty string? true
    Is '!' an empty string? true
    Is '0' an empty string? true
    
  • Erwin (unregistered)

    It's just a matter of the wrong function name, in both cases.

    It should have been isNotEmptyString, since they both return false when the string length is zero. (i.e. when the string is empty).

    The matter of the whitespace is good in both cases; since the method isn't documented, nobody can tell what is exactly meant by "empty" so both implementations are correct.

    So the real WTF is that the second developer didn't catch on to the real bug, being the name of the method.

  • hpeg (unregistered)

    When "".equals(obj) just isn't enough...

  • (cs)

    This would have been so much easier, and clearer in C ...

    g,d&r

  • been there (unregistered) in reply to hpeg
    hpeg:
    When "".equals(obj) just isn't enough...
    egads, man! It's short and efficient, actually performs the intended action and correctly handles null, string and non-string objects! How do you expect to make it on TDWTF when you code like that </smirk>
  • (cs) in reply to been there
    been there:
    if ( obj instanceof String ) { // if it's a string if ( ((String)obj).length() == 0 ) // and == "" return false; // then NOT emptyString - WTF?! }

    captcha = whiskey - mmm - need some...

    Yep. The function works bass-ackwards, not to mention it will return TRUE if I send it an Integer, an Object, or little green men.

    No, I doubt the function name's backward, the devel got it backwards. Or he's just done the "Russian Reversal" on his code, making it more WTFy than it already would be...


    In Soviet Russia, code writes YOU!!!

  • Troy McClure (unregistered)

    I dont get all these java examples. I guess this article would be a lot more interesting if I understood it.

    Oh well!

  • (cs)

    (x == null) followed by (x instanceof String) is redundant. instanceof implicitly checks against null.

  • Luciano Mollea (unregistered) in reply to hpeg

    yeah, my feeling exactly...

  • sriram (unregistered)

    is it java or is it .NET they all seem the same to me. i googled and found out it was java only. .NET uses typeOf.

  • Todd Hile-Hoffer (unregistered)

    If that is c# then you could use string.isNullOrEmpty. Why would anyone write that useless function that would make no sense even if .net didn't provide us with 10 million string objects already in the framework.

  • Chuggid (unregistered)

    return (obj instanceof String) && obj.toString().trim().length() == 0;

  • TSK (unregistered)

    Sorry for stating this, but i'm really not impressed with the quality of the comments...

    public boolean isEmptyString(CharSequence string) { return "".equals(string); }

    It automatically validates if the thing in question is a string-like thing, it allows StringBuilder,CharBuffer etc. and it returns false if string is null or not empty.

  • Damien Guard (unregistered)

    Microsoft gave us String.IsNullOrEmpty(obj) in .NET 2.0 so we don't make the same mistake...

    [)amien

  • Chuggid (unregistered) in reply to Chuggid

    Oh, actually, it looks like they don't want a whitespace string to count as an empty string. Weird.

    So, that makes it easy:

    return (obj instanceof String) && obj.toString().length() == 0;

  • Botia (unregistered)

    Ah, the classic boolean result is the opposite of what it should be in some cases bug. it doesn't get any better than this, especially when the function has been "fixed" and it still does that. Can anyone say "unit test?"

  • TSK (unregistered) in reply to TSK

    Upps, must be "return "".equals(string.toString())....

  • Chuggid (unregistered) in reply to TSK
    TSK:
    public boolean isEmptyString(CharSequence string) { return "".equals(string); }
    Doesn't handle the case of passing in, say, an Integer, which I take it the method is supposed to.

    I suppose you could do this:

    return String.valueOf(obj).equals("");

    ...and make the assumption that if an incoming Object's toString() method results in "" you should count it as an empty string too.

  • M Harris (unregistered)

    I think the WTF here is there was no unit test, which would have caught the original error.

  • TSK (unregistered) in reply to TSK

    Gosh !

    public boolean isEmptyString(CharSequence cs) { if (cs == null) return false; return "".equals(cs.toString()); }

    Sorry for risking a big lip...

  • (cs)

    if ( ((String)obj).length() == 0 ) return false;

    So.. if the string has length 0, isEmptyString returns false. but if it has length of, say, 1, isEmptyString returns true. wtf?

  • Best (unregistered)

    Clearly the correct function would return: (new math.Random()).nextBoolean() || obj == null;

    cause you can never go wrong with random

    captcha: awesomeness

  • (cs) in reply to Chuggid
    Chuggid:
    TSK:
    public boolean isEmptyString(CharSequence string) { return "".equals(string); }
    Doesn't handle the case of passing in, say, an Integer,
    Yes, it does. String.equals() (and pretty much any sane implementation of equals()) first checks the parameter's class using instanceof and returns false if it doesn't match.
  • JL (unregistered) in reply to brazzy
    brazzy:
    Chuggid:
    TSK:
    public boolean isEmptyString(CharSequence string) { return "".equals(string); }
    Doesn't handle the case of passing in, say, an Integer,
    Yes, it does. String.equals() (and pretty much any sane implementation of equals()) first checks the parameter's class using instanceof and returns false if it doesn't match.
    I haven't done Java in a while, but doesn't the parameter type "CharSequence" need to be "Object" for it to work as you suggest?
  • TSK (unregistered) in reply to Chuggid
    Chuggid:
    TSK:
    public boolean isEmptyString(CharSequence string) { return "".equals(string); }
    Doesn't handle the case of passing in, say, an Integer, which I take it the method is supposed to.

    I suppose you could do this:

    return String.valueOf(obj).equals("");

    ...and make the assumption that if an incoming Object's toString() method results in "" you should count it as an empty string too.

    You cannot enter an Integer because the method needs a CharSequence, meaning that you will get a compile error. I would even go further and don't allow a null at all:

    (Final)

    public boolean isEmptyString(CharSequence cs) { if (cs == null) throw new IllegalArgumentException("cs must not be null !"); return "".equals(cs.toString); }

    null don't make any sense at all for the method, so simply tell the programmer in no uncertain terms that he has done an error.

  • AndrewB (unregistered)

    "".equals(whatever)

    is technically not optimal because it unnecessarily wastes processor time constructing a string object ("") when one may very well already be constructed for us.

    Don't reinvent the wheel!

    String.Empty.equals(whatever)

    Also, here is the solution to the original solution

    public boolean isEmptyStringOhAndUseThisFunctionAndNotIsEmptyStringBecauseItIsBroken(Object obj) { return !isEmptyString(obj); }

  • hpeg (unregistered) in reply to AndrewB

    String.Empty is C#, rather than Java. Plus if your compiler doesn't optimise "" to String.Empty, then you have been ripped off...

  • Moogle (unregistered)

    Several people have commented that the code is fine other than the name reversal, yet no one had pointed out that both versions of the function return the same value for null and an empty string.

    WTF?

  • Wouter (unregistered)

    You should code it as follows

    bool IsStringEmpty(object o) { if(o is Int32) { return false; }else if (o is Int64) { return false; } .... abriviated long list of if statements

    return o is String && String.IsNullOrEmtpy(o); }

    Or create a factory pattern This greatly enhances my code line productivity :)

  • verisimilidude (unregistered) in reply to M Harris

    "I think the WTF here is there was no unit test, which would have caught the original error."

    Well at least he put it in a function. Too often I see tests like this in-line in some humungous function that goes on and on - totally too complex for any unit test. And I get told there is no time to refactor, just find THIS bug and fix it.

  • real_aardvark (unregistered) in reply to Todd Hile-Hoffer
    Todd Hile-Hoffer:
    If that is c# then you could use string.isNullOrEmpty. Why would anyone write that useless function that would make no sense even if .net didn't provide us with 10 million string objects already in the framework.

    Not to quibble about the "10 million string objects," which we can all understand as "string functions" or "string methods."

    I was trying to understand what the first attempt at IsStringEmpty was doing; then what the second attempt was doing, and why the programmer thought it better; then the usual flood of proposed "fixes" in the comments, all of which for all I know may be wonderful.

    But they miss the point. This stuff is FUBAR, as Obfuscator says way up above. I am especially grateful for his scratch unit test, which does rather cut through all the bull.

    Anyway. Todd -- I think inadvertently -- has the explanation for why FUBAR like this happens. Yes, .NET languages do indeed have ten million methods for string manipulation. (And I thought that the 96 or so for an STL string was a bit camel-comittee-y...) Can anyone say "minimal interface?"

    I mean, I understand that sometimes you want a non-mutable string. And sometimes you want a mutable string. And sometimes you want a string stream. Well, make the damn things Different Classes. Do NOT provide a gazillion stupid, overlapping routines for "string" manipulation, few if any of which are orthogonal... Otherwise you are simply bound to get idiots writing code like this.

    Mind you, the real WTF is that the company (a) hired this guy, presumably knowing he was crap (b) allowed him to (almost) put the first version into production without any sort of a test (c) asked him to rewrite it AND (d) did not insist on a unit test of any kind for what is clearly a general-purpose, low-level utility.

    The fact that the second version of the code makes my head spin is almost beside the point.

  • (cs)

    It is really unclear what the function is supposed to be doing... I'm not even sure the developer meant to be returning False for a Null value. Sure, null is not the same as an empty string, but where's the spec? I assume there is none... awesome! There's your WTF. Each of these functions appears to do something, probably wrong, based on what we assume a non-existent spec to state about what the function should do. I mean, as pointed out above, why not use a builtin function from StringUtils/String/etc?

  • 604 (unregistered)

    I think the a more serious wtf... I don't know if its already been posted, but why would they want such a method? when somoene else posted "".equals(object); works just as well?

  • Josh (unregistered) in reply to 604

    Or you could use a real language like .NET.

    String.Empty or String.IsNullOrEmpty statics anyone?

    Why reinvent something pretty much any nTier app would need. Specially when left to Java developers:P The results this blog makes painfully obvious every week from the comments alone.

  • (cs) in reply to verisimilidude
    verisimilidude:
    Too often I see tests like this in-line in some humungous function that goes on and on - totally too complex for any unit test. And I get told there is no time to refactor, just find THIS bug and fix it.

    But if you fix this function, you'll break all the code that uses it and works around the bug . Best solution is to rename the function to IsntNotANonEmptyString

  • (cs) in reply to Josh
    Josh:
    Or you could use a real language like .NET.

    String.Empty or String.IsNullOrEmpty statics anyone?

    Why reinvent something pretty much any nTier app would need. Specially when left to Java developers:P The results this blog makes painfully obvious every week from the comments alone.

    .Net doesn't prevent people from reinventing the wheel just like in any other language, in fact I shudder to think what VB.Net and C#.Net snippets we'll start seeing in here when these folks all start migrating over!

  • The validiator (unregistered) in reply to been there

    Shouldn't that be <smirk>? Where's the opening tag?? :P

    I only kid. I just couldn't resist.

  • Michael (unregistered) in reply to AndrewB
    AndrewB:
    "".equals(whatever)

    is technically not optimal because it unnecessarily wastes processor time constructing a string object ("") when one may very well already be constructed for us.

    If this is Java, then it is very likely that an instance of a string "" already exists in the internal string table, so this would not create a new object, it would just create a new pointer to the "" string in the table. This would be done at compile time, so there wouldn't even be any wasted CPU time when the method is called.

  • Zippy (unregistered) in reply to Josh
    Josh:
    Or you could use a real language like .NET.

    .NET is a language?

  • Michael (unregistered) in reply to Josh
    Josh:
    Or you could use a real language like .NET.

    String.Empty or String.IsNullOrEmpty statics anyone?

    Why reinvent something pretty much any nTier app would need. Specially when left to Java developers:P The results this blog makes painfully obvious every week from the comments alone.

    You mean like StringUtils.isEmpty()? Seriously, just because you don't know something exists, doesn't mean it doesn't exist.

  • Todd Hile-Hoffer (unregistered) in reply to real_aardvark

    I was just lazy and didn't feel like typing fuctions and methods.

    HAHA CAPTCHA Test is "initech"

  • Betta Glurpse (unregistered) in reply to sriram
    sriram:
    is it java or is it .NET they all seem the same to me. i googled and found out it was java only. .NET uses typeOf.
    Actually, the C# equivalent of java's "instanceof" is "is". And the java equivalent of C#'s typeof(x) is x.class
  • Ben Yogman (unregistered) in reply to hpeg

    They need a better function name, but they're trying to trim before comparison so that nice little snip won't quite do.

    So for the body of their function:

    // nulls are not instanceof anything if(obj instanceof String) { return ((String) obj).trim().length() == 0; } else { return false; }

Leave a comment on “Validating Nothing”

Log In or post as a guest

Replying to comment #111202:

« Return to Article