String Comparison
-
Another one found during code review. Constants changed to protect the innocent.
If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}
-
Another one found during code review. Constants changed to protect the innocent.
If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}
In the world of copy paste its easier to copy and pasting twice "CamelCasedName" and .ToUpper() than changing 3 character. mouse will talk ;)
-
Another one found during code review. Constants changed to protect the innocent.
If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}
Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive
ToUpper
calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:if ("CAMELCASENAME".equals(name.ToUpper())) {
...
}
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
-
Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive
ToUpper
calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:if ("CAMELCASENAME".equals(name.ToUpper())) {
...
}
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
Actually I completely disagree. == is not a major sin in this instance - it is fairly well understood that == is equivalent to an ordinal string comparison not an object equivalence. It is functionally equivalent to .Equals() and much more readable - I would pull up use of .Equals in a code review on the readability point. However the best option is to use String.Compare instead as this does proper culture sensitive comparisons.
-
Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive
ToUpper
calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:if ("CAMELCASENAME".equals(name.ToUpper())) {
...
}
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
Nagy Vilmos wrote:
you should always put the constant first as it removes the need for null checks
you have a point, and I always prefer '.Equals' especially with strings. Often this confuses the lexical parser of the compiler itself ;P Probleminha com 'if' C# - Operator '!='[^]
Regards Vallarasu S | FSharpMe.blogspot.com
-
Another one found during code review. Constants changed to protect the innocent.
If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}
This is how I would have done it:
if (string.Equals(name, "CamelCasedName", StringComparison.OrdinalIgnoreCase) == true)
{}
This looks more meaningful to me and i think it will be little better too(performance wise).
Rotted Frog wrote:
Constants changed to protect the innocent.
innocent :laugh:
Every now and then say, "What the Elephant." "What the Elephant" gives you freedom. Freedom brings opportunity. Opportunity makes your future.
-
Another one found during code review. Constants changed to protect the innocent.
If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}
-
Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive
ToUpper
calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:if ("CAMELCASENAME".equals(name.ToUpper())) {
...
}
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
I recommend this approach. The only time I use ToUpper or ToLower is with a switch. It's annoying that you can't use a caseless string comparer with replaces, but you can use a regular expression that is case insensitive for those.
-
This is how I would have done it:
if (string.Equals(name, "CamelCasedName", StringComparison.OrdinalIgnoreCase) == true)
{}
This looks more meaningful to me and i think it will be little better too(performance wise).
Rotted Frog wrote:
Constants changed to protect the innocent.
innocent :laugh:
Every now and then say, "What the Elephant." "What the Elephant" gives you freedom. Freedom brings opportunity. Opportunity makes your future.
-
Another one found during code review. Constants changed to protect the innocent.
If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}
I always wonder: why after many, many, many years people just can't learn to use caseless string comparison methods? I constantly see shit like this. Caseless comparison methods existed even in good old plain C. Damn, is it really that hard to master primitive things like this:
if (String.Compare(name, "CamelCasedName", true) == 0) {...}
? In the end, it's not Lebesgue integration, right? Those, who ever had the "pleasure" to be familiar with functional analysis, know :) .