current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

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.

share|improve this question
    
I think the second version is much better –  mariosangiorgio 8 hours ago
    
@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". –  Konrad Viltersten 8 hours ago
1  
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." –  Ben Aaronson 8 hours ago
    
Is this the real code, variable names, method names and all? If not there may be some context that we're missing. –  mjolka 8 hours ago
    
@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? –  Konrad Viltersten 8 hours ago

2 Answers 2

up vote 9 down vote

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.

share|improve this answer
    
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. :) –  Konrad Viltersten 8 hours ago
    
@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(). –  Magus 8 hours ago
    
@Magus Cool. I still want you to post your comment as a reply. –  Konrad Viltersten 8 hours ago
up vote 4 down vote

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.

share|improve this answer
    
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. –  mjolka 8 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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