CODEWITHSUNDEEP
c#.net
Maro
Maro

Reputation: 2629

Best practice for Inline if statement in for loop

I want to copy Listbox items to StringCollection. If the listbox items contains empty string then ignore

Why can i do this: 

foreach (string item in lstModelUsers.Items)
{
    if (string.IsNullOrEmpty(item))
        continue;
    else
        Options.Default.ModelRemoveUsers.Add(item);
}

BUT not this:

foreach (string item in lstModelUsers.Items)
    string.IsNullOrEmpty(item)
        ? continue
        : Options.Default.ModelRemoveUsers.Add(item);

Although both appear equal, the inline if statement generates a syntax error.
What is the best practice?

Upvotes: 2

Views: 2218

Answers (4)

Mark Byers
Mark Byers

Reputation: 838276

You can't use the conditional operator like that. It only accepts expressions as its operands. Your code fails to compile because continue can only be used as a statement, not an expression.

A better approach is to negate the if expression so that you don't need the continue:

foreach (string item in lstModelUsers.Items)
{
    if (!string.IsNullOrEmpty(item))
    { 
        Options.Default.ModelRemoveUsers.Add(item);              
    }
}

You could also use Where:

var itemsToAdd = lstModelUsers.Items
    .Cast<string>()
    .Where(item => !string.IsNullOrEmpty(item));

foreach (string item in itemsToAdd)
{
    Options.Default.ModelRemoveUsers.Add(item);   
}

If you are lucky, you may even find that ModelRemoveUsers has an AddRange method, then you don't need the loop at all:

var itemsToAdd = lstModelUsers.Items
    .Cast<string>()
    .Where(item => !string.IsNullOrEmpty(item));

Options.Default.ModelRemoveUsers.AddRange(itemsToAdd);

Upvotes: 8

Grhm
Grhm

Reputation: 6834

I'd use LINQ:

foreach (string item in lstModelUsers.Items.Where(user => !string.IsNullOrEmpty(user))
{
    Options.Default.ModelRemoveUsers.Add(item);
}

And depending on the type of ModelRemoveUsers you could make it into a single line

Options.Default.ModelRemoveUsers.AddRange(
    lstModelUsers.Items.Where(user => !string.IsNullOrEmpty(user));

But I prefer Mark Byers non-single line version as it is more readable and thus will be easier to maintain over time.

Upvotes: 3

Hans Kesting
Hans Kesting

Reputation: 39283

No, they are not equal. The conditional operator (? :) requires the two result parts to end up in a value (continue doesn't have a value). Plus both values need to be of a similar type.

Upvotes: 2

Oded
Oded

Reputation: 499042

You can't use continue within the conditional operator.

The two "branches" of the conditional operator need to return the same type or types that can be implicitly converted to each other.

Upvotes: 2

Related Questions