current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 5 down vote favorite

Can the below foreach loop be improved? Since I use the same method twice, maybe use continue or break?

int? index = null;
int count = 0;
foreach (Break b in breaks)
{
    if (index.HasValue)
    {
        if (index == count)
            b.Remove();
    }
    else
    {
        b.Remove();
    }
    count++;
}

So I turned it into this:

foreach (Break b in breaks)
{
    if ((index.HasValue && index == count) || !index.HasValue)
    {
        b.Remove();
    }
    count++;
}

Which is the same. Any other optimizations possible?

share|improve this question
4  
index is always null, so why not simply remove it? int count = 0; foreach(Break b in breaks { b.Remove(); count++ }? What value does having index serve? – Eric Lippert Sep 12 '13 at 17:06
2  
I’ve always wondered why so many languages lack a Boolean “implies” operator a ==> b equivalent to (a && b) || !a, or perhaps let x = a in (x && b) || !x in the presence of side effects. Moreover, where’s my let-in expression? – Jon Purdy Sep 12 '13 at 19:40
up vote 15 down vote accepted

How about removing the loop at all like:

if (!index.HasValue)
{
     breaks.Clear();
}
else
{
     breaks.RemoveAt((int)index);
}

if you want/need to keep the loop, i would change your second way like:

foreach (Break b in breaks)
{
    if (!index.HasValue || index == count)
    {
        b.Remove();
    }
    count++;
}
share|improve this answer
up vote 11 down vote

Leaving aside the question of why you have index in the first place, you don't need to check for nullity before you use a comparison operator. If you have 

int x = whatever;
int? y = whatever;
bool b = x == y;

Then b will be true if y has a value equal to x and false otherwise. You don't have to say something like:

bool b = y.HasValue ? x == y.Value : false;

The compiler will generate that code on your behalf. This feature is called lifting to nullable and it applies to most of the operators in C#.

If the subject of how the compiler analyzes and generates code for lifted operators interests you, I wrote a long series of articles explaining it in detail. See

http://ericlippert.com/2012/12/20/nullable-micro-optimizations-part-one/

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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