|
It gets worse. When I escape the quotes to script an update of that field, I get the SQL below. The reason for all the casts is historical, and the reason for all the quotes is that the SQL gets passed through several layers of stored proc before actually being executed.
UPDATE [csForm] SET [CopyCommand] = N'SELECT @ValueList = ''''''''+REPLACE(ISNULL(CAST([Amount] as varchar(8000)),''Null''),'''''''','''''''''''') + '''''','''''' + REPLACE(ISNULL(CAST([DocTitle] as varchar(8000)),''Null''),'''''''','''''''''''') + '''''' FROM Documents WHERE DocID =|SELECT ''''''&IDENTITY&'''','''''' + REPLACE(ISNULL(CAST([WorkType] as varchar(8000)),''Null''),'''''''','''''''''''') + '''''','''''' + REPLACE(ISNULL(CAST([WorkTypeID] as varchar(8000)),''Null''),'''''''','''''''''''') + '''''','''''' + REPLACE(ISNULL(CAST([Amount] as varchar(8000)),''Null''),'''''''','''''''''''') + '''''','''''' + REPLACE(ISNULL(CAST([Notes] as varchar(8000)),''Null''),'''''''','''''''''''') + '''''','''''' + REPLACE(ISNULL(CAST([LineType] as varchar(8000)),''Null''),'''''''','''''''''''')+'''''''' AS ValueList FROM DocItems WHERE DocID =|'
|
|
|
|
|
Brady Kelly wrote: The reason for all the casts is historical ...
Don't you mean hysterical
|
|
|
|
|
this is why they created linq !
|
|
|
|
|
You have my utmost condolences...
|
|
|
|
|
Hi, I saw this some time ago:
for(int i = 0; i < 2; i++)
{
if(i==0)
if(i==1)
}
this is pretty much a horror
|
|
|
|
|
Your are correct, he/she should have used a switch instead of the if statements. Like this:
for(int i= 0; i < 2; i++)
{
switch(i)
{
case 0:
break;
case 1:
break;
};
}
Learn from the mistakes of others, you may not live long enough to make them all yourself.
|
|
|
|
|
I guess there is no need for a loop at all.
Nothing is being repeated here.
«_Superman_»
I love work. It gives me something to do between weekends.
|
|
|
|
|
«_Superman_» wrote: Nothing is being repeated here.
We don't know that.
|
|
|
|
|
I believe the point was that without the loop and the if's the (equivalent) code becomes
// do something
// do something else
which of course is simply normal program flow! So the whole looping business seems a bit convoluted.
|
|
|
|
|
Probably, but do //Do something and //Do something else alter the value of i ?
|
|
|
|
|
PIEBALDconsult wrote: Probably, but do //Do something and //Do something else alter the value of i?
I don't see how they can...
|
|
|
|
|
{
i=i-2;
Console.WriteLine("once more");
}
Luc Pattyn [Forum Guidelines] [My Articles]
- before you ask a question here, search CodeProject, then Google
- the quality and detail of your question reflects on the effectiveness of the help you are likely to get
- use the code block button (PRE tags) to preserve formatting when showing multi-line code snippets
|
|
|
|
|
Such code may be entirely reasonable and practical if there is common code before or after the case-specific code, and if variable scoping would make it impractical for the common code to be pulled out into a separate routine.
My guess would be that there had at some point been some code which was executed every time through the loop, but that such code has been eliminated leaving the now-useless loop structure behind.
|
|
|
|
|
This is for sure a Coding Horror, a good one.
Does a loop for only two predefined values. If i (counter) is changed inside for, this is another coding horror!. And guess what, the developer is interesting for the values 0 and 1. (reminds you something like true/false maybe?)
|
|
|
|
|
I agree with the poster who said it depends.
If this is the loop's entire construct - yes I think it's not ideal. (Not a horror though). I would just have two statements "do something" and "do something else" and MAYBE block them in braces to be clear its a "unit" or make a function if warranted.
If the person planned to expand on this loop - where it cycled thru ten iterations, but was suppose to do something special for iteration 1 and 2, then a case statement might be ideal.. still not a horror.
--Jason P Sage
Know way to many languages... master of none!
|
|
|
|
|
The fact is that it was on a "finished" code, so the programmer didn't want to expand it. I agree with the idea that if you have a two iterations for and have two different actions on each, then there shouldn't be any for at all!
|
|
|
|
|
Do something absolutely is critical here to know whether it is horrible or not.
The following sample indicates what you saw is not horrible:
See, I have two controls: Label0 and TextBox1. I can assign values to them:
for (int i = 0; i < 2; i++)
{
if (i == 0)
{
Label lable = (Label)this.FindControl("Label" + i.ToString());
if (lable != null)
lable.Text = "Test Label " + i.ToString();
}
if (i == 1)
{
TextBox txt = (TextBox)this.FindControl("TextBox" + i.ToString());
if (txt != null)
txt.Text = "Test TextBox " + i.ToString();
}
}
See it? It is not a usual way, but, not as horrible as you saw.
modified on Monday, March 2, 2009 4:04 PM
|
|
|
|
|
Jokin' i suppose...
|
|
|
|
|
I've been reading programming books for quite a while now, and I agree this is totally useless even for explaining the syntax of the if statement. For goodness sake couldn't they write something like this at least:
for(int i = 0; i < 2; i++)
{
if(i==0)
Console.WriteLine("This is a zero");
if(i==1)
Console.WriteLine("This is a one");
}
So you could kind of understand that the code supposed to do something. In this case write something to the screen. And if you wrote this into your visual studio it would actually work!!!
|
|
|
|
|
I once saw a programmer write this.
<br />
<br />
foreach (User user in users)<br />
{<br />
return user;<br />
}<br />
<br />
|
|
|
|
|
If users provides only the IEnumerable interface then you kinda have to do that.
We'd need to see more of the code to determine just how horrible it is.
|
|
|
|
|
|
That's an Extension Method, not a member of IEnumerable, and therefore has no access to the underlying data; it is likely implemented the same way as the original post.
|
|
|
|
|
PIEBALDconsult wrote: likely implemented the same way as the original post.
Which was exactly my point, i think
|
|
|
|
|
J4amieC wrote: IEnumerable.FirstOrDefault[^] aside of course.
If, in fact, you are using .NET 3.5, as otherwise this doesn't exist.
|
|
|
|