5
\$\begingroup\$

This review is very short and very straight-forward. There are two people in the team and the checked-in code looks as follows.

string parameter = GetUnitType();
string target = "PM4000";
if(parameter.CompareTo(target) == 0)
  CarryOutProcess();

I got stuck here and needed to re-check the usage of CompareTo. I can't for my life understand any advantage of the above in comparison to:

string parameter = GetUnitType();
if(parameter == "PM4000")
  CarryOutProcess();

I'm pretty sure it's just a case of intentional obfuscation or, at the very least, lack of understanding a copied code.

Is the former inferior to the latter in every possible way?

The author of the first claims that since both work, they're comparably good and the difference is just a matter of personal taste.

\$\endgroup\$
10
  • \$\begingroup\$ I think the second version is much better \$\endgroup\$
    – mariosangiorgio
    Commented Sep 24, 2014 at 22:10
  • \$\begingroup\$ @mariosangiorgio So do I. And, probably 99% of others. The problem is that the person who suggests the first way know as much of programming as penguins about flying but he insists on putting his two cents in because he can't assess his own ignorance. I need a bunch of answers supporting my claims and, preferably, a stone-hard motivation why these aren't just "two different ways of doing the same thing". \$\endgroup\$
    – Konrad Viltersten
    Commented Sep 24, 2014 at 22:13
  • 2
    \$\begingroup\$ From msdn.microsoft.com/en-us/library/35f0x18w%28v=vs.110%29.aspx : "The CompareTo method was designed primarily for use in sorting or alphabetizing operations. It should not be used when the primary purpose of the method call is to determine whether two strings are equivalent. To determine whether two strings are equivalent, call the Equals method." \$\endgroup\$
    – Ben Aaronson
    Commented Sep 24, 2014 at 22:14
  • \$\begingroup\$ Is this the real code, variable names, method names and all? If not there may be some context that we're missing. \$\endgroup\$
    – mjolka
    Commented Sep 24, 2014 at 22:16
  • \$\begingroup\$ @BenAaronson But that can be questioned since the equality between strings is generally done by double equality signs. I don't want the dilettante writing .Equals() instead. I want him to use "==" operator. That is the correct and accepted method, is it? \$\endgroup\$
    – Konrad Viltersten
    Commented Sep 24, 2014 at 22:18

3 Answers 3

Reset to default
7
\$\begingroup\$

Another difference is illustrated in the following code:

string str1 = null;
string str2 = "hello!";

Console.Write(str1 == str2);
Console.Write(str1.CompareTo(str2));

Using == will run if the left operand is null, calling CompareTo will not.

So if in your posted code:

string parameter = GetUnitType();

GetUnitType() returned null and CompareTo was called, you would get a NullReferenceException. If you used ==, this exception would not be raised.

\$\endgroup\$
1
  • \$\begingroup\$ This is a perfect suggestion! I fear that, although the other replies are more sophisticated and have a larger weight information-wise (hitting on the general case of comparisons beyond the limited scope of String class), they'll miss the target because the developer in question is way to stubborn and arrogant to realize their own incompetence. I doubt he will check the docs, let alone understand his mistake. However, your example kicks the issue straight in the groin. I think this will be my accepted answer (still appreciating the others as great too.) +1 and green-check. \$\endgroup\$
    – Konrad Viltersten
    Commented Sep 25, 2014 at 13:55
16
\$\begingroup\$

There is a subtle difference between == and CompareTo: the latter is current-culture-aware (or you can pass another one in), the former is not.

However, as Ben points out in the comments, the official recommendation is to use Equals instead of CompareTo outside of sorting operations.

Your devs might also be coming from a Java background where == and equals are a bit more complicated and may very well produce different results. This isn't a concern in C#, so for the vast, vast majority of your equality checks the == will be more than sufficient.

\$\endgroup\$
3
  • \$\begingroup\$ As for the culture (+1), we only compare an all-anglo-saxian unit name with a set of possible three other all-anglo-saxian names. There's no chance whatsoever that the culture will cause issues. As for the origins of the pseudo-developer, it's not Java. It's actually nothing at all. And I'm almost sure that he copies some code without understanding why and how it's supposed to be used. But he claims that it's one man's opinion against another. I claim it's his opinion against most developers. I just need to check with "the most" to be sure. :) \$\endgroup\$
    – Konrad Viltersten
    Commented Sep 24, 2014 at 22:25
  • \$\begingroup\$ @KonradViltersten: Developers' opinions won't win that. Microsoft's, as the designer of the language, might. He could find some arbitrary forum that supports him otherwise. So, you can only win with .Equals(). \$\endgroup\$
    – Magus
    Commented Sep 24, 2014 at 22:27
  • \$\begingroup\$ @Magus Cool. I still want you to post your comment as a reply. \$\endgroup\$
    – Konrad Viltersten
    Commented Sep 24, 2014 at 22:31
11
\$\begingroup\$

Anna Lear has pointed out flawlessly that CompareTo is meant for sorting and similar operations, rather than equality.

In .NET, most of the time you can indeed get away with the standard == operator, safely assuming it has been overridden to perform some sort of object comparison. If it hasn't, it will be reference comparison. This is also true of Equals.

Generally speaking, the == operator is overridden in a way which just calls Equals, as is the case for the String class.

However, Equals has some other advantages. Firstly, not everyone chooses to override operators, meaning that Equals may well be the only correct way to compare. This, of course, is not true of the String class. There is another reason though: as a method, it can be overloaded with more arguments. For String, this includes culture and case comparison, which can be very valuable. This is probably a large part of Microsoft's suggestion to prefer it for string equality.

As for the particular situation you find yourself in, a battle of opinion won't be won by finding more people who agree with you, as anyone can do that. The best route is to use the word from on high: Microsoft, in this case. Ultimately, however, the person you are subjecting to this crusade will probably never thank you for this, and the friction may not be beneficial.

Do think about that.

\$\endgroup\$
2
  • \$\begingroup\$ Very nice final point. A simple "don't do it that way, do it this way" should suffice; answer any follow-up questions and finally point to the docs. \$\endgroup\$
    – mjolka
    Commented Sep 24, 2014 at 23:00
  • \$\begingroup\$ I'm torn now. Both this and the first reply is great, they complete each other and each is an answer to my question on its own... Sometimes I wish for a primary and a secondary "Oh, yeah!"-button to click when two replies are both answers... And seem already to experience so much friction that a bit more won't do any more damage. (+1 for the great advice, though.) :) \$\endgroup\$
    – Konrad Viltersten
    Commented Sep 25, 2014 at 13:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.