Comment On Compare.java

Tom G. recently joined a team that maintained a fairly large Java client/server application. His first task was fairly simple: prepare for a switch to a different server farm by going through the code to make sure it was portable and wouldn't be affected by a new server, IP address, and so on. [expand full text]
« PrevPage 1 | Page 2Next »

Re: Compare.java

2010-07-19 09:12 • by frits
This kind of code makes me miss C-style macros.

Re: Compare.java

2010-07-19 09:13 • by Anon (unregistered)
comment == "frist" ? "frist" : (String) null;

Re: Compare.java

2010-07-19 09:15 • by Drew (unregistered)
Is the WTF the code itself or the fact that if either option is null, it returns true?

Re: Compare.java

2010-07-19 09:15 • by Spivonious (unregistered)
Wow...it's as if Java didn't have String comparisons built into the language...

Re: Compare.java

2010-07-19 09:23 • by wtf (unregistered)
314781 in reply to 314779
Drew:
Is the WTF the code itself or the fact that if either option is null, it returns true?


It's an asymmetrical equals. If s1 is null and s2 is not null, return true. If s1 is not null and s2 is null, return false.

Re: Compare.java

2010-07-19 09:30 • by JavaConfig (unregistered)
Wow, never did Java, but surely, to move from one server to another, developers do not need to go through each line of code. What ever happened to write once run everywhere?

Switching server farms should only involve looking at config files IMO.

Re: Compare.java

2010-07-19 09:40 • by grzlbrmft (unregistered)
314784 in reply to 314781
wtf:
Drew:
Is the WTF the code itself or the fact that if either option is null, it returns true?


It's an asymmetrical equals. If s1 is null and s2 is not null, return true. If s1 is not null and s2 is null, return false.


So the real WTFs are:
- what you said
- that the method comment is not a Javadoc comment
- nested ternary operators (resulting in reduced readability)
- unnecessary casts (resulting in reduced readability)



What about those (also without Javadoc comments ;-)):


public static boolean equalsAllowNull(Object o1, Object o2){
return o1 == null || o2 == null || o1.equals(o2);
}

public static boolean equalsAllowNull(String s1, String s2){
return s1 == null || s2 == null || s1.equalsIgnoreCase(s2);
}

Re: Compare.java

2010-07-19 09:43 • by Anonymous (unregistered)
This is an error from Firefox users. What the hell did you do to break the layout in Firefox?

Looks like your HTML meta-comment might be responsible since none of the HTML after this point is being rendered. I'm posting this message from IE6 (excuse me while I take a shower, I feel dirty).

Re: Compare.java

2010-07-19 09:44 • by Alex (unregistered)
314786 in reply to 314782
That implies the code is sane in the first place. You wouldn't believe the number of developers who think that hard-coded strings are an acceptable form of configuration.

Re: Compare.java

2010-07-19 09:44 • by kennytm
TRWTF is whoever use "--" inside a comment tag.


<!--
this -- is wrong
-->
things here will be commented out
-->

Re: Compare.java

2010-07-19 09:50 • by Rob (unregistered)
Please fix your HTML comments for those of us that use a real browser. I'm not enjoying this re-visit of IE6.

Re: Compare.java

2010-07-19 09:55 • by Steve (unregistered)
Awesome meta-WTF guys, better than today's article IMO.

<!-- Gotta watch out for those HTML comments! -->

Re: Compare.java

2010-07-19 09:57 • by washii (unregistered)
314790 in reply to 314782
Wow, never did Java, but surely, to move from one server to another, developers do not need to go through each line of code. What ever happened to write once run everywhere?

Switching server farms should only involve looking at config files IMO.


Well, you have to WRITE your code to take advantage of that. If you hard-code server names and IPs into your code, it doesn't matter about 'Write Once.' It will only 'run everywhere you have the exact same servernames/IPs'.

Re: Compare.java

2010-07-19 10:03 • by frits
314792 in reply to 314790
washii:
Wow, never did Java, but surely, to move from one server to another, developers do not need to go through each line of code. What ever happened to write once run everywhere?

Switching server farms should only involve looking at config files IMO.


Well, you have to WRITE your code to take advantage of that. If you hard-code server names and IPs into your code, it doesn't matter about 'Write Once.' It will only 'run everywhere you have the exact same servernames/IPs'.


Bah, whatever! Just use find/replace and run the new server in a test environment for a while. You should be able to find and fix most of the bugs. Then throw it over the wall and let the users report whatever else you missed. The introduction of new server should be an exciting event!

Re: Compare.java

2010-07-19 10:05 • by Patrick (unregistered)
314793 in reply to 314778
Anon:
comment == "frist" ? "frist" : (String) null;


this is an error from postComment() method only accepts meaningful sentences, what the hell did you do?!

Re: Compare.java

2010-07-19 10:21 • by Jellineck (unregistered)
314794 in reply to 314788
Rob:
Please fix your HTML comments for those of us that use a real browser. I'm not enjoying this re-visit of IE6.


Not everybody wants to use a 'real browser' or even cares to install Lynx.

Re: Compare.java

2010-07-19 10:29 • by C (unregistered)
314795 in reply to 314787
kennytm:
TRWTF is whoever use "--" inside a comment tag.

<!--
this -- is wrong
-->
things here will be commented out
-->
Wtf are you talking about? "--" is quite correct inside comments (look it up!), it's supposed to separate adjacent comments from each other, not that i've ever found this useful.

However, your having two "-->" is TRWTF...

Re: Compare.java

2010-07-19 10:34 • by kennytm
314796 in reply to 314795
C:
kennytm:
TRWTF is whoever use "--" inside a comment tag.

<!--
this -- is wrong
-->
things here will be commented out
-->
Wtf are you talking about? "--" is quite correct inside comments (look it up!), it's supposed to separate adjacent comments from each other, not that i've ever found this useful.

However, your having two "-->" is TRWTF...


If it's correct than it won't break Firefox's layout. In fact, "--" is disallowed in comments in HTML 5[1] and XML[2].

[1]: http://www.w3.org/TR/html5/syntax.html#comments
[2]: http://www.w3.org/TR/REC-xml/#sec-comments

Re: Compare.java

2010-07-19 10:40 • by jdw (unregistered)
314798 in reply to 314788
Rob:
Please fix your HTML comments for those of us that use a real browser. I'm not enjoying this re-visit of IE6.
Chrome isn't a real browser? :(

Re: Compare.java

2010-07-19 10:54 • by jpers36
Wow, comments are broken bad. Half of them seem to be missing, and what's with the new colors?

Re: Compare.java

2010-07-19 10:57 • by veggen
314800 in reply to 314798
jdw:
Rob:
Please fix your HTML comments for those of us that use a real browser. I'm not enjoying this re-visit of IE6.
Chrome isn't a real browser? :(

Nope. Only Opera qualifies ;)
(Half)joke aside, I see no problems in Opera...

Re: Compare.java

2010-07-19 11:00 • by sstair
Getting back to the subject at hand - What is wrong with writing something that saves you having to explicitly check for null before calling string1.equalsIgnoreCase(string2)?

Re: Compare.java

2010-07-19 11:04 • by Jan (unregistered)
314802 in reply to 314781
Well, it wasn't /intended/ to be asymmetrical, if I read the comment correctly; and it's the thought that counts really, isn't it? :)

Re: Compare.java

2010-07-19 11:05 • by sstair
Ahh, I see the problem. Nothing wrong with the intent, it is the execution that is flawed.

Re: Compare.java

2010-07-19 11:09 • by PS (unregistered)
314804 in reply to 314784
[quote user="grzlbrmft]What about those (also without Javadoc comments ;-)):


public static boolean equalsAllowNull(Object o1, Object o2){
return o1 == null || o2 == null || o1.equals(o2);
}

public static boolean equalsAllowNull(String s1, String s2){
return s1 == null || s2 == null || s1.equalsIgnoreCase(s2);
}
[/quote]

Almost. I think the correct version would be:


public static boolean equalsAllowNull(Object o1, Object o2){
return (o1 == null && o2 == null) || o1.equals(o2);
}

public static boolean equalsAllowNull(String s1, String s2){
return (s1 == null && s2 == null) || s1.equalsIgnoreCase(s2);
}


But seriously, where is the WTF? He wants to compare two strings, both of which may or may not be NULL, without cluttering his main code with IFs. It's clumsily written alright but still a valid cause. In any case not nearly enough to pull an "enough is enough".

P.S.:
You don't have to abandon all morals and use IE6. IE8 will do just as well.

P.P.S.: jumentum.

Re: Compare.java

2010-07-19 11:09 • by Anon (unregistered)
So I don't "do" Java, but I would have assumed that (s1 instanceOf string) would return false is s1 was null, and (s1 instanceOf Integer) would also return false is s1 is null (how can null be either a string or an integer?). So, therefore, if s1 actually is null, you'll get an error. So the function actually doesn't allow null at all.
Am I wrong?

Also, the comment "allow me to compare strings and Integers" would lead me to think you could compare 1 string to 1 integer, which this function also doesn't do.

Re: Compare.java

2010-07-19 11:14 • by Anon (unregistered)
314807 in reply to 314805
Anon:
So I don't "do" Java, but I would have assumed that (s1 instanceOf string) would return false is s1 was null, and (s1 instanceOf Integer) would also return false is s1 is null (how can null be either a string or an integer?). So, therefore, if s1 actually is null, you'll get an error. So the function actually doesn't allow null at all.
Am I wrong?

Also, the comment "allow me to compare strings and Integers" would lead me to think you could compare 1 string to 1 integer, which this function also doesn't do.


Quick Google search and it looks like I'm right:

http://www.java2s.com/Tutorial/Java/0060__Operators/TheinstanceofKeyword.htm

However, applying instanceof on a null reference variable returns false. For example, the following if statement returns false.


So if s1 and/or s2 are actually null, this function won't return true, it'll give you an error.

The "tweaked" method, I think, does at least actually work.

Re: Compare.java

2010-07-19 11:18 • by Kensey
314808 in reply to 314796
kennytm:
If it's correct than it won't break Firefox's layout.


Right, all versions of Firefox are 100% compliant with all DTDs.

Re: Compare.java

2010-07-19 11:23 • by tharpa (unregistered)
314809 in reply to 314785
Anonymous:
This is an error from Firefox users. What the hell did you do to break the layout in Firefox?

Looks like your HTML meta-comment might be responsible since none of the HTML after this point is being rendered. I'm posting this message from IE6 (excuse me while I take a shower, I feel dirty).


I am reading and writing this from Firefox. It looks fine to me. I wonder what the difference is?

Re: Compare.java

2010-07-19 11:23 • by Someone You Know
314810 in reply to 314805
Anon:
So I don't "do" Java, but I would have assumed that (s1 instanceOf string) would return false is s1 was null, and (s1 instanceOf Integer) would also return false is s1 is null (how can null be either a string or an integer?). So, therefore, if s1 actually is null, you'll get an error. So the function actually doesn't allow null at all.
Am I wrong?

Also, the comment "allow me to compare strings and Integers" would lead me to think you could compare 1 string to 1 integer, which this function also doesn't do.


Correct. In Java, null is never instanceof anything.

Re: Compare.java

2010-07-19 11:25 • by fnord (unregistered)
314811 in reply to 314804
PS:
Almost. I think the correct version would be:


public static boolean equalsAllowNull(Object o1, Object o2){
return (o1 == null && o2 == null) || o1.equals(o2);
}

public static boolean equalsAllowNull(String s1, String s2){
return (s1 == null && s2 == null) || s1.equalsIgnoreCase(s2);
}



Almost. If o1 is null and o2 is not null, you'll have a NullPointerException.

public static boolean equalsAllowNull(String s1, String s2){
return (s1 == null && s2 == null) || (s1 != null && s1.equalsIgnoreCase(s2));
}

But seriously, where is the WTF? He wants to compare two strings, both of which may or may not be NULL, without cluttering his main code with IFs. It's clumsily written alright but still a valid cause. In any case not nearly enough to pull an "enough is enough".

Well, it does not do what it advertizes (i.e. the whole "s1 is null s2 is not null results in true" story mentioned above), and the first version with the funny casting behavior is, in my book, a pretty resonable "get me out of here" moment.

Re: Compare.java

2010-07-19 11:26 • by Ken B. (unregistered)
314812 in reply to 314780
Spivonious:
Wow...it's as if Java didn't have String comparisons built into the language...
Or integer compares, either.

Re: Compare.java

2010-07-19 11:29 • by Markp
314813 in reply to 314804
PS:

Almost. I think the correct version would be:


public static boolean equalsAllowNull(Object o1, Object o2){
return (o1 == null && o2 == null) || o1.equals(o2);
}



if ( equalsAllowNull(null, Integer.valueOf(6)) ) { //crash!


The ternary operator is correct here, but the WTFer got it wrong. You need


public static boolean equalsAllowNull(Object o1, Object o2){
return o1 == null ? o2 == null : o1.equals(o2);
}


Since a compliant equals method will always return false for a null argument.

Re: Compare.java

2010-07-19 11:32 • by Colin (unregistered)
Wow, how is everyone coming up with such crap null-safe equals methods?


public boolean equal(Object o1, Object o2) {
return o1 == o2 || (o1 != null && o1.equals(o2));
}

Re: Compare.java

2010-07-19 11:34 • by Dan (unregistered)
TRWTF = "We are currently narrating this post."

Re: Compare.java

2010-07-19 11:43 • by EngleBart (unregistered)
314816 in reply to 314811
Another WTF, the comments mention:

... for which i write test cases works like a regular equalsIgnoreCase except ...

The developer obviously did not write the simplest test cases to validate the building block of some more complicated test cases.

assertTrue(equalsAllowNull(null, null));
assertFalse(equalsAllowNull(null, "a"));
assertFalse(equalsAllowNull("a", null));
assertFalse(equalsAllowNull("a", "b"));
// new() to prevent compiler from reusing same instance...
assertFalse("a" == new String("a"));
// make sure code is using .equals and not ==
assertTrue(equalsAllowNull("a", new String("a")));

Re: Compare.java

2010-07-19 11:44 • by Anonymous (unregistered)
314817 in reply to 314809
tharpa:
Anonymous:
This is an error from Firefox users. What the hell did you do to break the layout in Firefox?

Looks like your HTML meta-comment might be responsible since none of the HTML after this point is being rendered. I'm posting this message from IE6 (excuse me while I take a shower, I feel dirty).


I am reading and writing this from Firefox. It looks fine to me. I wonder what the difference is?

No difference, the problem has now been fixed by an editor.

Re: Compare.java

2010-07-19 11:57 • by The_Assimilator
TRWTF:

System.out.println("strcmp 1:"+s1+" 2:"+s2);

This is what happens when you let bad C programmers use Java - they try to make Java behave the way they're used to, as opposed to learning to work with it, and write the stupidest, shittiest code as a result.

Another symptom of this: writing error messages to the console instead of throwing an exception when you get unexpected input.

Re: Compare.java

2010-07-19 12:43 • by Rick
This common problem and many like them have already been solved. http://commons.apache.org/lang/

public static boolean StringUtils.equalsIgnoreCase(String str1, String str2)

Its 2010, people. TRWTF is thinking that you have to write this method yourself.

Re: Compare.java

2010-07-19 12:49 • by Console Cop (unregistered)
314820 in reply to 314818
The_Assimilator:
TRWTF:

System.out.println("strcmp 1:"+s1+" 2:"+s2);

This is what happens when you let bad C programmers use Java - they try to make Java behave the way they're used to, as opposed to learning to work with it, and write the stupidest, shittiest code as a result.

Another symptom of this: writing error messages to the console instead of throwing an exception when you get unexpected input.
But if we don't write to the console, how will the console operator ever know the batch job had an abend, and he needs to go get the deck of cards and feed them thru the reader again?

Re: Compare.java

2010-07-19 13:00 • by Coyne
314821 in reply to 314784
grzlbrmft:
wtf:
Drew:
Is the WTF the code itself or the fact that if either option is null, it returns true?


It's an asymmetrical equals. If s1 is null and s2 is not null, return true. If s1 is not null and s2 is null, return false.

(omitted text)

So the real WTFs are:
- what you said
- that the method comment is not a Javadoc comment
- nested ternary operators (resulting in reduced readability)
- unnecessary casts (resulting in reduced readability)

(more omitted text)



Nope. That's not it either. In Java, null does not have a type and so the instanceof will not test true even if you send two so-called "String" objects. So if either of the inputs (or both) is null, then the result is the message at the bottom.

Utterly broken!

Re: Compare.java

2010-07-19 13:05 • by Jay (unregistered)
314822 in reply to 314814
Colin:
Wow, how is everyone coming up with such crap null-safe equals methods?


public boolean equal(Object o1, Object o2) {
return o1 == o2 || (o1 != null && o1.equals(o2));
}


I say give Colin the cookie! This is almost identical to what I write, the minor difference being that I typically say:

return o1==o2 || o1!=null && o2!=null && o1.equals(o2);

Testing o2 against null is not necessary for any of the standard classes, which will cleanly return false if the value compared against is null. But you can't necessarily be sure about user-defined classes.

P.S. There is a lot of code in the system I'm working on these days that goes quite beserk about this, with code like:

boolean equalsIgnoreNull(Object o1, Object o2)
{
if (o1==null)
{
if (o2==null)
return true;
else
return false;
}
else if (o2==null)
{
return false;
}
else
{
if (o1.equals(o2))
return true;
else
return false;
}
}

Sometimes they'll retest o1 inside the o2==null test, I guess just in case the first o1==null test gave the wrong answer.

Re: Compare.java

2010-07-19 13:07 • by Jay (unregistered)
Oops, I used the wrong "quote code" convention on my previouis post. I was thinking I was on Stack Overflow. Sorry for the ugly formatting, but hopefully you get the idea. Or don't care.

Re: Compare.java

2010-07-19 13:10 • by Jay (unregistered)
314824 in reply to 314782
JavaConfig:
Wow, never did Java, but surely, to move from one server to another, developers do not need to go through each line of code. What ever happened to write once run everywhere?

Switching server farms should only involve looking at config files IMO.


The operative word here being "should". People "should" respect each other's property, but I still lock my doors at night.

Re: Compare.java

2010-07-19 13:19 • by Markp
314825 in reply to 314822
Jay:
Testing o2 against null is not necessary for any of the standard classes, which will cleanly return false if the value compared against is null. But you can't necessarily be sure about user-defined classes.


Who the hell cares about user-defined classes with bugs in them? If they want your function to work with them, they can conform to the specification for equals, which explicitly states that anyNotNullRef.equals(null) must return false.

Re: Compare.java

2010-07-19 13:29 • by edthered (unregistered)
314826 in reply to 314796
kennytm:
C:
kennytm:
TRWTF is whoever use "--" inside a comment tag.

<!--
this -- is wrong
-->
things here will be commented out
-->
Wtf are you talking about? "--" is quite correct inside comments (look it up!), it's supposed to separate adjacent comments from each other, not that i've ever found this useful.

However, your having two "-->" is TRWTF...


If it's correct than it won't break Firefox's layout. In fact, "--" is disallowed in comments in HTML 5[1] and XML[2].

[1]: http://www.w3.org/TR/html5/syntax.html#comments
[2]: http://www.w3.org/TR/REC-xml/#sec-comments


TRWTF is specs written (by) for lazy browser vendors. --> ends a comment, not - or -- or -> or >. I should be able to put whatever I want in a comment as long as it's not <!-- or --> and the browser should render it correctly. (actually after the first <!-- any subsequent <!-- should be ignored in the same comment block)

What would happen to block comments in java/etc. if you couldn't have a * or a / in them? Or if an extra / made the line comment not a comment?

Re: Compare.java

2010-07-19 14:08 • by доод (unregistered)
314827 in reply to 314790
washii:
Wow, never did Java, but surely, to move from one server to another, developers do not need to go through each line of code. What ever happened to write once run everywhere?

Switching server farms should only involve looking at config files IMO.


Well, you have to WRITE your code to take advantage of that. If you hard-code server names and IPs into your code, it doesn't matter about 'Write Once.' It will only 'run everywhere you have the exact same servernames/IPs'.


In that case, whatever happened to using config files that store that kind of server-specific information?

Re: Compare.java

2010-07-19 14:11 • by THG (unregistered)
Maybe if the author had heard Tom loud enough,
they could've written code proofread enough.

Re: Compare.java

2010-07-19 14:30 • by future guy (unregistered)

public static boolean equalsAllowNull(String s1, String s2){
System.out.println("strcmp 1:"+s1+" 2:"+s2);
return (s1 != null)
?((s2 != null)
?(((String)s1).equalsIgnoreCase(((String)s2)))
:false)
:true;
}

Okay, someone please tell me I'm blind and I've missed something and explain why does s1==null return true? Also what's the point of casting string into string ? sure it looks cool with all those brackets, but it still hurts my eyes.

I know this is the daily wtf, but having that in a "fairly large Java client/server application" someone had to see the error sooner!

CAPTCHA: uxor - upper case XOR, used to compare strings and integers!

Re: Compare.java

2010-07-19 15:02 • by marcan (unregistered)
TRWTF is that it's 2010 and Java still needs a method call to test for object equality. In real languages, you use == to compare objects (or at least strings) and null == null (or whatever term the language uses for null)
« PrevPage 1 | Page 2Next »

Add Comment