Is returning a functions value from a for-loop bad practice in C++?

Question

My colleague just had a look at my code and said, that according to "some standard" returning a functions value out of a for loop is bad practice.

The function looks something like this:

bool CentralWidget::isBitFieldFree(const QString& define, int lsb, int msb)
{
    QString defineWithoutIndex = getDefineWithoutIndex(define);
    for (int i = lsb; i <= msb; i++)
    {
        if ((registerBitMap[defineWithoutIndex] >> i) & 1)
            return false; //Returning here early...
        else
            registerBitMap[defineWithoutIndex] |= 1 << i;
    }
    return true;
}

Questions:

1. Is there a a standard that bans this?
2. is this concidered bad practice?
3. if so: why?

Answer 1

There is no such standard as this in the world of c++. However in a more specific environment (such as a company), specialized standards might apply.

With that said, it's not bad practice, in general.

---

a) a standard that bans this?

No.

b) is this concidered bad practice?

No.

c) question is already answered.

---

Other than that, it lies under the scope of personal preference, and here is mine - but this obviously not a part of a real answer:

You want to return true, after you have looped over your string, so how could you do it elegantly inside the loop? With an if statement saying that if it's the last iteration, return true, at the end of the loop?

I think this puts more lines of code in your file, without any reason, and damaging severely the readability.

Answer 2

There can be different opinion on this and people may argue it makes an "unintuitive" control flow. But there is no standard for this.

Personally I find this a better practice than having an extra variable and condition to exit the code. You want to leave the scope at this point and you do this by returning from the function. I sometimes even put code in an extra function just to be able to use the feature of leaving the scope from anywhere with a return.

This also has the advantage of creating less lines of code. And less lines of code are less opportunities to introduce a bug.

Answer 3

There is a school of thought that says that a function should have a single entry and a single exit point. That makes some static analysis of the code easier, but in C++ very likely serves no other purpose.

This kind of thinking comes mostly from C, where it can make a lot of sense to exit the function trough a single point to ensure resources eventually taken by the function are properly cleaned up on exit. Replicating this cleanup across multiple exits is considered to be brittle and hard to maintain - I agree.

In C++ however the preferred way to control your resources is through RAII or by using RAII-classes like shared pointers, string and vector. Thus the amount of code required to clean up resources on exit is often null, because it is implicitly done in the destructors of the objects in the local scope. Therefor it usually does not make any sense to enforce single-entry/single-exit unless you have a good reason to do so.

Answer 4

In some coding standards you could find things like 'a function should have only one exit point'. Often this was related to cleanup procedures, but in the days of smart pointers this does not apply anymore.

So, as the other answers and comments say: No, there is no such standard. Also, I don't think it's bad practice.

Even more: I would say you don't need the 'else', because if you exit the loop/function, processing only continues in the 'else' case. Does not matter much in your case, but if you have multiple ifs, it can save you a lot of nesting.