Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This question already has an answer here:

Recently I've been messing around with this. Note that I'm not using LINQ because while this could be easily be done that way, I could do this in any language.

Allow me to exemplify:

foreach ( var i in items )
{
    if ( condition )
    {
        ...
    }
}

This could be read as: "for each item that meets condition, do ..."

Let's just say that ... is several lines long. In order for me to know that that this code is equivalent to the statement above, I have to ensure that there is nothing after said if. I'd rather write it this way:

foreach ( var i in items ) 
    if ( condition )
{
    ...
}

That said, since this particular case is rather brief, I could also write it this way:

foreach ( var i in items ) if ( condition )
{
    ...
}

And let's just say ... is a single, short statement. This could all fit into a single line without looking jarring at all.

var x = new List<Foo>()
foreach ( var i in items ) if ( condition ) x.Add(i);

Which, in LINQ, would be:

x = items.Where( i => condition );

Remarkably similar.

Another interesting example, would be when I'm nesting foreach loops to look for something.

foreach ( var i in foos ) foreach ( var j in i.bars )
{
    //Do Stuff With Bars
    ...
}

Contextually, the first foreach is tied to the second. What I'm actually interested in are the bars, not the foos. I think writing it this way reinforces that idea.

This is, essentially, the same thing we do when we chain if blocks, which should technically look like this:

if ( a )
{
    ...
}
else
{
    if ( b )
    {
        ...
    }
}

Whereas, everyone I know, myself included, writes:

if ( a )
{
    ...
}
else if ( b )
{
    ...
}

I am not trying to take shortcuts or write less lines, but simply group things together because it makes sense for them to be together on the same line.

share|improve this question

marked as duplicate by MichaelT, gnat, Bart van Ingen Schenau, GlenH7, Yusubov Jun 6 '13 at 13:02

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

2  
What's your question? –  Karl Bielefeldt Jun 5 '13 at 20:12
    
Whether this is an acceptable practice or not, in your opinion, and why. –  elite5472 Jun 5 '13 at 20:14

1 Answer 1

The vast majority of programmers will immediately recognize

foreach ( var i in items )
{
    if ( condition )
    {
        ...
    }
}

for what it is. It is an immediately recognizable form.

None of the other forms are immediately recognizable as such, and therefore automatically disqualify as good practice. They create just enough cognitive dissonance that any advantage of brevity they might have is immediately negated by the extra few seconds it takes the programmer's mind to parse them.

By your own admission,

if ( a )
{
    ...
}
else if ( b )
{
    ...
}

is different, because everyone already writes it this way.

This form has a compelling reason to exist, where the others do not: it is a variant of switch case that has the same form (all of the conditional statements have the same indentation), except that the conditions can be more sophisticated than a switch case.

It makes more sense than

if ( a )
{
    ...
}
else
{
    if ( b )
    {
        ...
    }
}

because this form suggests that one of the if statements has a higher "ranking" than the other, when in actual fact, they do not. Programmers recognize the first form for what it is: a more sophisticated switch statement.

The only shorthand I could get on board with is this:

foreach (var item in list.Where(condition))
{
}

This form acknowledges the fact that Linq doesn't have a Foreach<T>() extension method. The reason it doesn't is because Linq statements are supposed to be side-effect free, and ForEach<T> would exist only to produce side-effects.

I'm on the fence with

foreach ( var i in items ) if ( condition )
{
    ...
}

This actually looks elegant, and I don't think any developer would have trouble understanding what you did.

This, on the other hand

foreach ( var i in items ) if ( condition ) x.Add(i);

is just an abomination, because there's no closing brace.

share|improve this answer
    
Thanks for the answer, you do have a point about familiarity. I don't completely agree that familiarity should take precedence over simplicity, however. For example, in what manner is foreach ( var i in items ) if ( condition ) x.Add(i); any less of an abomination than the LINQ equivalent? And what about cases where LINQ isn't there to begin with, such as other languages? If the statement and the condition are really short, it's actually fairly easy to read, and it's purpose is clearly conveyed. –  elite5472 Jun 6 '13 at 13:28
    
Linq is basically a Fluent Interface; everyone knows what that is. foreach ( var i in items ) if ( condition ) x.Add(i); is not; it's just several statements mashed onto the same line for the sake of less braces. –  Robert Harvey Jun 6 '13 at 14:46

Not the answer you're looking for? Browse other questions tagged or ask your own question.