|
CDP1802 wrote: copied and pasted to do the validation for another date
Only such far yet? Things will become much nicer in near future, when a fix is introduced here, another (different) fix there, some extra validation elsewhere, and soon you'll have some ten (different) solutions for the same problem (and none working correctly).
Seems to be a wide-spread pattern. Highly contagious, terribly infectious...
|
|
|
|
|
This developer is the Dark Lord and master of all techniques that make code unmaintainable. I have the dubious honor of having inherited all projects he ever did. Perhaps I should give him a special award for his lifetime achievements and personally shake his neck.
At least artificial intelligence already is superior to natural stupidity
|
|
|
|
|
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
|
|
|
|
|
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
|
|
|
|