Using an IF statement inside loop vs outside the loop

Question

i have a program with a nested nested nested loop (four loops altogether). I have a Boolean variable which i want to affect the code in the deepest loop and in the first nested loop a small amount. My dilemma is, i don't really want to have the if else statement put inside the loops as i would have thought that would check the Boolean's state every iteration using extra time to check the statement, and i know that the Boolean's state would not change when the loop starts. This lead me to think it would be better to place the if else statement outside the loops and just have my loops code slightly changed, however, this also looks messy there is a lot of repeated code.

One thing i thought might work, but of which i have little experience using, is a delegate, i could simply put some of the code in a method and then create a delegate, depending on the state of betterColor i could then assign that delegate methods with the different code on them beforehand, but this also seems messy.

Below is what i am trying to avoid as i thhought it may slow down my algorithm:

for (short y = 0; y < effectImage.Height; y++)
{
    int vCalc = (y <= radius) ? 0 : y - radius;

    for (short x = 0; x < effectImage.Width; x++)
    {
        int red = 0, green = 0, blue = 0;
        short kArea = 0;

        for (int v = vCalc; v <= y + radius && v < effectImage.Height; v++)
        {
            int calc = calcs[(y - v) + radius];
            for (int h = (x <= calc || calc < 0) ? 0 : x - calc; h <= x + calc && h < effectImage.Width; h++)
            {
                if (betterColor == true)
                {
                    red += colorImage[h, v].R * colorImage[h, v].R;
                    green += colorImage[h, v].G * colorImage[h, v].G;
                    blue += colorImage[h, v].B * colorImage[h, v].G;
                    kArea++;
                }
            }
        }

        if (betterColor == true)
            effectImage.SetPixel(x, y, Color.FromArgb(red / kArea, green / kArea, blue / kArea));
        else
            effectImage.SetPixel(x, y, Color.FromArgb(Convert.ToInt32(Math.Sqrt(red / kArea)), Convert.ToInt32(Math.Sqrt(green / kArea)), Convert.ToInt32(Math.Sqrt(blue / kArea))));
    }

    if (y % 4 == 0) // Updates the image on screen every 4 y pixels calculated.
    {
        image.Image = effectImage;
        image.Update();
    }
}

And here is what my code now looks like:

if (betterColor == true)
{
    for (short y = 0; y < effectImage.Height; y++)
    {
        int vCalc = (y <= radius) ? 0 : y - radius;

        for (short x = 0; x < effectImage.Width; x++)
        {
            int red = 0, green = 0, blue = 0;
            short kArea = 0;

            for (int v = vCalc; v <= y + radius && v < effectImage.Height; v++)
            {
                int calc = calcs[(y - v) + radius];
                for (int h = (x <= calc || calc < 0) ? 0 : x - calc; h <= x + calc && h < effectImage.Width; h++)
                {
                    red += colorImage[h, v].R * colorImage[h, v].R;
                    green += colorImage[h, v].G * colorImage[h, v].G;
                    blue += colorImage[h, v].B * colorImage[h, v].G;
                    kArea++;
                }
            }

            effectImage.SetPixel(x, y, Color.FromArgb(Convert.ToInt32(Math.Sqrt(red / kArea)), Convert.ToInt32(Math.Sqrt(green / kArea)), Convert.ToInt32(Math.Sqrt(blue / kArea))));
        }

        if (y % 4 == 0) // Updates the image on screen every 4 y pixels calculated.
        {
            image.Image = effectImage;
            image.Update();
        }
    }
}
else
{
    for (short y = 0; y < effectImage.Height; y++)
    {
        int vCalc = (y <= radius) ? 0 : y - radius;

        for (short x = 0; x < effectImage.Width; x++)
        {
            int red = 0, green = 0, blue = 0;
            short kArea = 0;

            for (int v = vCalc; v <= y + radius && v < effectImage.Height; v++)
            {
                int calc = calcs[(y - v) + radius];
                for (int h = (x <= calc || calc < 0) ? 0 : x - calc; h <= x + calc && h < effectImage.Width; h++)
                {
                    red += colorImage[h, v].R;
                    green += colorImage[h, v].G;
                    blue += colorImage[h, v].B;
                    kArea++;
                }
            }

            effectImage.SetPixel(x, y, Color.FromArgb(red / kArea, green / kArea, blue / kArea));
        }

        if (y % 4 == 0) // Updates the image on screen every 4 y pixels calculated.
        {
            image.Image = effectImage;
            image.Update();
        }
    }
}

In terms of what the code does, it is a box blur that uses a circular kernal.

Answer 1

Moving the if out of the loop, and effectively duplicating the whole looping code is not really worth it. So if you have code like this:

for (i …)
{
    if (something)
        DoX(i);
    else
        DoY(i);
}

Then you should not replace it by this:

if (something)
{
    for (i …)
        DoX(i);
}
else
{
    for (i …)
        DoY(i);
}

Doing so will only make the code a lot more difficult to read and to maintain. One first need to figure out that this is actually the same code that’s being executed for each case (except that one tiny difference), and it’s super difficult to maintain once you need to change anything about the loop since you need to make sure that you edit both cases properly.

While in theory, performing a single check vs. performing that check N times is obviously faster, in practice, this rarely matters. If-branches that rely on a constant boolean are super fast, so if you calculate the condition outside of the loop (in your case betterColor is set outside the loop), then the performance difference will not be noticeable at all. In addition, branch prediction will usually make sure that there is no difference at all in these cases.

So no, don’t rewrite that code like that. Keep it the way that is more understandable.

In general, you should avoid these kind of micro optimizations anyway. It is very likely that you algorithm has much slower parts that are much more relevant to the overall performance than small constructs like that. So focusing on those small things which are already very fast will not help you make the total execution faster. You should only optimize things that are an actual performance bottleneck in your code, where a profiler showed that there is a performance issue or that optimizing that code will actively improve your performance. And stay away from optimizations that make code less readable unless you really need it (in most cases you won’t).

Answer 2

The way i see it, the code is not equivalent (in your first example if betterColor is false, nothing happens in the innermost loop).

But isn't this micro-optimizing?

You could probably do something with creating a function with a Func<> as argument for the innermost loop. And then pass the correct func depending on the betterColor value.

I.E. Blur(betterColor?FuncA:FuncB); Although I don't think it will be faster then a boolean check... But that's my feeling.