Loop antipattern
-
I've heard of this antipattern but never actually encountered it before. Well, until today.
@WTF Code said:for (int i = 0; i < 6; i++)
{
if (i == 0)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
else if (i == 1)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
else if (i == 2)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
else if (i == 3)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
else if (i == 4)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
else if (i == 5)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
else if (i == 6)
{
foreach (var item in dbItems)
{
// Equality checks
}
}
}
Of course there are no comments anywhere, leading to questions like "Why six?" Also that "i == 6" block will never execute. Variable names are word-salad, the authors of this code were not native English-speakers so I think they picked random English words to make the project look more English.
It appears to be searching a database for some values. The odd thing is immediately before this section, they used LINQ to do some database queries.
Other WTFs included
- Hard-coded file and DLL references to a certain developer's "My Documents" folder
- Unused DLL references that would cause lots of exceptions if they were used because Silverlight and the full .NET Framework use different binary assembly formats
- The delivered project did not compile at all, I had to add an #endregion to almost every single C# file, plus some right curly braces to a number of files to close the class
- There is absolutely no authentication or security of any type so anybody anywhere would be able to nuke the server-side database by using the built-in application features
- Hard-coding lots of stuff that really should be pulled from a config file which means this plugin works ONLY in the test environment they were given. We have 60+ clients with independent instances of our application with the only difference being in the text-based config files, so hard-coding that stuff is clearly against our coding standards.
- Unhandled runtime exceptions which prevent the site from loading. It appears the code requires the test data to be in the database, else it blows up.
Disclaimer: This was student code, written as a plugin to our existing product. However it came from a graduate-directed project at our local university and the students fully expected their code would be used in production. Internally we expected to use it as a bare functional prototype to get some R&D done before we build the final product, but I thought it would be interesting to see what their code quality would be if they thought it would go into production use.
-
@mott555 said:
Ah, the legendary FOR-CASE paradigm.I've heard of this antipattern but never actually encountered it before. Well, until today.
-
@DaveK said:
More like FOR-CASE-FOR-EACH paradigm in this case.@mott555 said:
Ah, the legendary FOR-CASE paradigm.I've heard of this antipattern but never actually encountered it before. Well, until today.
-
Havign a for each inside each of thses isn't that bad, as long as it is just the different processign for each number.
Otherwise this is even worse!
-
@KattMan said:
Havign a for each inside each of thses isn't that bad, as long as it is just the different processign for each number.
But then the outer for loop is completely unnecessary.
-
@morbiuswilters said:
@KattMan said:
Havign a for each inside each of thses isn't that bad, as long as it is just the different processign for each number.
But then the outer for loop is completely unnecessary.Yeah maybe depending on what they are for, if it is child records they are needed and the outer loop is good.
If it is the same record, then yes do the for-each outside the stupid switch implementation. That's why I said it could be worse.
-
@KattMan said:
@morbiuswilters said:
@KattMan said:
Havign a for each inside each of thses isn't that bad, as long as it is just the different processign for each number.
But then the outer for loop is completely unnecessary.Yeah maybe depending on what they are for, if it is child records they are needed and the outer loop is good.
If it is the same record, then yes do the for-each outside the stupid switch implementation. That's why I said it could be worse.
Unless i is being modified within the loop itself (which would be another big WTF) then you can literally just remove the outer loop and if statements. It's just executing each block in sequence, just as if there were no loop to begin with.
-
@morbiuswilters said:
Unless i is being modified within the loop itself (which would be another big WTF)
They were probably told not to use gotos.
-
@morbiuswilters said:
Unless i is being modified within the loop itself (which would be another big WTF)
#define goto(N) i = N; continue
-
for - if - foreach
WE MUST GO DEEPER
...into Morbs quivering body! Also that code could humorously do with some more nesting of control structures because that's always funny.
-
@morbiuswilters said:
Unless i is being modified within the loop itself (which would be another big WTF)
This WTF is called a Deterministic Finite State Automaton. Look it up.
Of course, that usually involves modifying i only within the loop and not in the third part of the for statement.
-
You guys are ascribing way too much cleverness to the authors of the code. i is only modified by the loop control. As morbs said, you can remove the outer loop and the if-statements and it will run exactly the same way.
-
@Medinoc said:
This WTF is called a Deterministic Finite State Automaton. Look it up.
No, it's not. There's absolutely nothing nothing nothing in the OP that would make anyone think that this was a DFA and you look foolish for suggesting otherwise.
-
@morbiuswilters said:
@Medinoc said:
This WTF is called a Deterministic Finite State Automaton. Look it up.
No, it's not. There's absolutely nothing nothing nothing in the OP that would make anyone think that this was a DFA and you look foolish for suggesting otherwise.
Welllll...
- It is deterministic. We can tell exactly what it will do, in what order, and how many extra times it will do it.
- It is finite. It repeats itself only a limited number of times, and is guaranteed to terminate (barring external influence).
- It is stateful. All states are exactly the same, and it always goes through all of them, but it does have them.
- It is an automaton. Manual intervention is not required after starting it.
Is it a useful DFA? Not in the slightest. Is it actually one? Techincally, yes.
-
@pkmnfrk said:
Is it actually one? Techincally, yes.
First off: I'm going to punch you in the balls.
Second: you have no way of knowing this since some of the code is elided. It's entirely possible the program never terminates. If you're going to be a pedantic dickweed, make sure you're right.
-
@morbiuswilters said:
@pkmnfrk said:
Is it actually one? Techincally, yes.
First off: I'm going to punch you in the balls.
Second: you have no way of knowing this since some of the code is elided. It's entirely possible the program never terminates. If you're going to be a pedantic dickweed, make sure you're right.
Mott did say that i is only modified by the loop header. I think that's why Pokemon dared your wrath by responding - he knew his pedantic dickweedery was right. I don't know why I'm daring your wrath. I guess I'm an idiot.
hides
and protects balls
-
-
@dhromed said:
@aihtdikh said:
and protects balls
You're a lesbian and have balls?
Someone else's, worn as earrings.
-
@Cassidy said:
@dhromed said:
Ben Wa's?@aihtdikh said:
Someone else's, worn as earrings.and protects balls
You're a lesbian and have balls?
-
@morbiuswilters said:
If you're going to be a pedantic dickweed, make sure you're right.
Are you trying to kill off the entire forum?
-
Fun fact:
Postscript doesn't have a switch statement. This structure (without the else's and the foreach's) is one of a handful of ways to emulate them, and the only way to emulate them in a way that sort of looks vaguely like a switch statement (the others have more in common with reflection). Or you could have stupid deep nested if-else blocks (else if also doesn't exist because of syntactic issues)
I personally prefer the reflection-alike method, but occasionally do the for-if when I just need one-liners.
-
@Weng said:
Fun fact:
Postscript doesn't have a switch statement. This structure (without the else's and the foreach's) is one of a handful of ways to emulate them, and the only way to emulate them in a way that sort of looks vaguely like a switch statement (the others have more in common with reflection). Or you could have stupid deep nested if-else blocks (else if also doesn't exist because of syntactic issues)
I personally prefer the reflection-alike method, but occasionally do the for-if when I just need one-liners.
Fun fact:
Java doesn't have a useful switch statement, despite supposedly being a modern language..
-
@morbiuswilters said:
I vaguely recall that as well. Doesn't it only switch on numerics? Maybe even only integers?
Java doesn't have a useful switch statement, despite supposedly being a modern language..
-
Ints, enums and (in 7) strings.
-
@Kittemon said:
@morbiuswilters said:
They may take oor pedantic! But they'll never take oor DICKWEED!If you're going to be a pedantic dickweed, make sure you're right.
Are you trying to kill off the entire forum?
