|
I'm reading a 790 line SQL sproc, a lot of poor coding peppered with extrainious begin/end statements. It starts early on with a "begin tran" lots of poor inserts and updates. Looks like SQL 2000 code, no try/catch logic, then the kicker:
End
IF @@ERROR <> 0
-- error statements using the first needed begin/end block
Else
Commit Tran
End
That was the first reference in the whole novel to @@ERROR! (Code created for Msft)
For those who don't know SQL, that "End" before the "IF" FORCES it to always try to Commit the transaction, no matter how many errors the user has hit by now.
|
|
|
|
|
I have seen something like that. A big problem if it's in a high security thing.
return true;
|
|
|
|
|
Sounds like it was written by someone who doesn't really know SQL. They are only worth keeping around if they can be trained to do it the right way. If they argue that their way is perfectly fine, then chuck them out the door.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
One developer at work like to do things the hard way.
protected void ProcessDropDown(DropDownList myCombo, string myValue)
{
foreach (ListItem myItem in myCombo.Items)
{
if (myItem.Value == myValue)
{
myItem.Selected = true;
break;
}
}
}
All this just to select an item on aspx pages, should i throw him by the window?
And yes, this method is used only on the .cs file. So why use protected and do this thing? My eyes still hurt.
return true;
|
|
|
|
|
There are times when I am glad that 'the hard way' is still available. For one thing I don't like to be confined to using the one and only 'right way' to do things. Data binding may be the 'easy way', but also creates its own new problems. Just srearch for 'GridView' in Q&A. Besides that, data binding can be unacceptably slow in some cases. Filling controls directly can speed up things very much. And when I decide to do it 'the hard way' I also tend to keep all eggs in one basket and fill all controls that way, just to minimize the potential for misunderstandings.
Luckily, I have not been thrown by the window for doing that yet.
Edit: 'protected' is probably not the best choice in this case, but it does not do any harm.
At least artificial intelligence already is superior to natural stupidity
modified 10-May-12 3:22am.
|
|
|
|
|
|
This seems like he just didn't know about SelectedItem ... a harmless mistake, easily corrected.
|
|
|
|
|
That would have been too easy. But I think I came across some control without SelectedItem or SelectedIndex properties.
At least artificial intelligence already is superior to natural stupidity
|
|
|
|
|
Hopefully, that guy can be taught the correct way to do things. If not, then it is time to take him to that window.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
CIDev wrote: take him to that window
At least post a sign at the window: "This is the window out of which you will be thrown if you write bad code."
|
|
|
|
|
Found this little nugget just now (only the name of the stored procedure was changed):
Dim strSelect As String = "exec SomeStoredProcedure " & 1
|
|
|
|
|
Well, at least it's not vulnerable to SQL injection...
|
|
|
|
|
I suspect the original was, and the constant was put in there when a variable was no longer required, but they didn't want to put in the extra effort of placing the constant inside the string.
|
|
|
|
|
Could have been worse. It could have been C# and then you would have to do
String strSelect = "exec SomeStoredProcedure " + "1";
At least VB allows the 1 to be concatenated into the string.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
ryanb31 wrote: At least VB allows the 1 to be concatenated into the string.
Which is completely terrible that it allows you to do that. I also came across some code the other day that said something like this:
Dim str As String = "blah" & i + 1
In C#, you'd do this:
string str = "blah" + (i + 1).ToString();
Which I think is much more understandable.
|
|
|
|
|
Implicit conversions works equally well in VB and C#, AFAIK.
I would obviously use paratheses and ToString in VB as well, for clarity purposes.
It's basically as easy to code badly in C# as it is in VB.
In the end it's up to you.
|
|
|
|
|
C# does not implicitly convert integers to strings.
|
|
|
|
|
Well I'll be a monkey's uncle. I just tried it and C# does allow integers to be implicitly converted to strings.
Oh well, at least it does not allow the reverse.
|
|
|
|
|
|
Yeah, I can't find a reference to this anywhere aside from online forums. It would be neat if there's a way to disable it (like VB's option strict).
|
|
|
|
|
"In C#, you'd do this:"
Well i guess this is much better you know
string s = "blah" + ((i<<1)- --i);
modified 13-May-12 5:56am.
|
|
|
|
|
Uh, what. No you don't. String concatenation in C# automatically calls ToString on non-string arguments, including basic types. I use this quite frequently for generating temporary debug output.
|
|
|
|
|
I want to throw my computer off the roof. I'm modifying a 700+ line stored procedure and this is done a dozen+ times.
( CASE WHEN [SomeID] IN ( [batch of hardcoded, unique ids] )
THEN [hardcoded enum]
ELSE ( CASE WHEN [SomeID] IN ( [batch of hardcoded, unique ids] )
THEN [hardcoded enum]
ELSE ( CASE WHEN [SomeID] IN ( [batch of hardcoded, unique ids] )
THEN [hardcoded enum]
ELSE …etc…
END )
END )
END )
I …just…
|
|
|
|
|
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 I’ll probably get confronted for changing it. (I’m the new guy in the group.)
I think I’ll adopt a new saying: if you don’t respect my genius then you’ll fear my delete key!
|
|
|
|