|
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.
|
|
|
|
|
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  Probleminha com 'if' C# - Operator '!='[^]
|
|
|
|
|
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 
Every now and then say, "What the Elephant." "What the Elephant" gives you freedom. Freedom brings opportunity. Opportunity makes your future.
|
|
|
|
|
Are you sure to use
if(Equals() == true) why not
if((Equals()==true) == true) ? Isn't this better:
if(Equals()) ?
|
|
|
|
|
It's a Copy and Paste victim.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
I always wonder: why after many, many, many years people just can't learn to use caseless string comparison methods? I constantly see sh*t 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  .
|
|
|
|
|
pt = dsPubs.Tables(0)
appt = dsPubs.Tables(1)
GridView1.DataSource = dsPubs.Tables(0)
GridView1.DataBind()
GridView2.DataSource = dsPubs.Tables(1)
GridView2.DataBind()
|
|
|
|
|
Confusion... otherwise known as job security... 
|
|
|
|
|
He was just putting the DataTables off to the side in case he needed them later. You never know when you might need a good DataTable again.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
Just a quick one, but amused me when I saw it in code today.
if (i == 0)
return 1;
else
return i + 1;
|
|
|
|
|
What was that coder thinking?
EDIT: Or WAS he/she thinking?
public class SysAdmin : Employee
{
public override void DoWork(IWorkItem workItem)
{
if (workItem.User.Type == UserType.NoLearn){
throw new NoIWillNotFixYourComputerException(new Luser(workItem.User));
}else{
base.DoWork(workItem);
}
}
}
|
|
|
|
|
And let those who have never failed to see the obvious throw the first stone
At least artificial intelligence already is superior to natural stupidity
|
|
|
|
|
public class SysAdmin : Employee
{
public override void DoWork(IWorkItem workItem)
{
if (workItem.User.Type == UserType.NoLearn){
throw new NoIWillNotFixYourComputerException(new Luser(workItem.User));
}else{
base.DoWork(workItem);
}
}
}
|
|
|
|
|
Hey now, maybe it was originally written with constants or enumerations. 
|
|
|
|
|
yeah, sure, increment a enumeration...
I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p)
|
|
|
|
|
That is funny++.
"You get that on the big jobs."
|
|
|
|
|
Interesting
But i can not imagine the reason of the code. Can you give the full scope of code? Thinking but nothing find about the code
|
|
|
|
|
A nice piece indeed.
Here is a slightly more defensive version that makes sure the sign is properly handled:
if (i < 0)
return 1 - abs(i);
else if (i == 0)
return 1;
else if (i > 0)
return 1 + abs(i);
(with the added benefit that out-of-range values are left unchanged)
modified 19-Jun-12 2:22am.
|
|
|
|
|
return (i < 0) ? (1 - abs(i)) : ((i == 0) ? 1 : 1 + abs(i));
Here. Shorter now, and less obvious to spot. The benefits of multiple ternaries
|
|
|
|
|
Right. This allows us to move the common constant in front and factor out the abs call:
return 1 + abs(i) * ((i < 0) ? - 1 : ((i == 0) ? 0 : + 1));
But how do we make the i > 0 case explicit ???
Maybe
return 1 + abs(i) * ((i < 0) ? - 1 : ((i == 0) ? 0 : ((i > 0) ? + 1 : abort(), 0)));
|
|
|
|
|
And even better, we can abstract away the "1", who knows, maybe its value will change somewhere in the future:
final int _CONST = 1;
return _CONST + abs(i) * ((i < 0) ? - _CONST : ((i == 0) ? 0 : ((i > 0) ? + _CONST : abort(), 0)));
Can I have that mind bleach now, please? 
|
|
|
|
|
|
Hey but what about i being sqrt(2)???
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
