|
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.
|
|
|
|
|
Ouch, you sure stepped in the salad!
That code necessarily returns the first item in enumerating "users". Had it been a yield return things would be different though.
|
|
|
|
|
Ummm... yeah, so?
|
|
|
|
|
What is horrible about giving preferential treatment to the first user?
By the time the app has more users, they probably will have introduced a new keyword yielding some attention to more recent customers...
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
|
|
|
|
|
Just as long as it isn't alphabetical...I got tired of being at the end of the line in Elementary
|
|
|
|
|
You're sure it wasn't something like this?
foreach (User user in users)
{
yield return user;
}
Marc
|
|
|
|
|
Wow, I didn't know about the yield operator till now. Thanks for bringing this to my attention.
As for whether the code itself had a yield operator, it was so long ago I cannot remember. I'll give the programmer the benefit of the doubt.
|
|
|
|
|
I think Luc may have been hinting at this - he'd better remember not to make such hints so subtle next time...
"they probably will have introduced a new keyword yielding some attention to more recent customers..."
|
|
|
|
|
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
|
|
|
|
|
|
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
|
|
|
|
|
hey 10x for teling abt yield operator
Ravie Busie
Coding is my birth-right and bugs are part of feature my code has!
|
|
|
|
|
I have seen this when the array doesn't contain a indexing property.
return users[0]; // Doesn't work
I seem to remember this being the default behaviour for non generic Lists in early .net (2?).
|
|
|
|