Search code examples
coding-styleindentationconventions

Should indentation always be minimized?


I would like to hear your opinion regarding to whether it is good to minimize the indentation.

This is how I usually do it, to handle problems:

int foo_a() {
    if (!check_value(x)) {
        // error
        return false;
    }
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    return true;
}

On the other hand, I also saw such code:

int foo_b() {
    if (!check_value(x)) {
        // error
        return false;
    } else {
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        return true;
    }
}

int foo_c() {
    if (check_value(x)) {
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        return true;
    } else {
        // error
        return false;
    }
}

But this is probably contraproductive since the idents would become very large, if every check will create a new else-branch.

On the other hand, for decisions, e.g. vegetable or meat, I usually do it this way:

int foo_d(FOOD food) {
    if (food.isVegetable) {
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        return;
    } else {
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        return;
    }
    // assume here is NO shared code which is always executed for both food types.
}

But doing it like the way foo_a() does, it should look like this:

int foo_e(FOOD food) {
    if (food.isVegetable) {
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        // do stuff
        return;
    }

    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    // do stuff
    return;
}

Solution

  • I personally believe that both

    if(flag) {
        //long computation
        return;
    } else {
        //long computation
        return;
    }
    

    as well as

    if(flag) {
        //long computation
        return;
    }
    //long computation
    return;
    

    are antipatterns, because they make it more difficult to reason about program flow and the possible return values. They also more likely result in errors during refactoring - or in general during later modifications, as one might overlook the first return statement.

    For that reason, some coding guidelines allow only one return statement per function. In this case you will always have to use if and else:

    int foo(int param) {
        int retval = 0;
        if (param > 0) {
            //computation
            retval = 5;
        } else {
            //computation
            retval = -1;
        }
        return retval;
    } 
    

    Personally I usually allow two areas where return statements are allowed: Right at the beginning for abnormal or trivial returns (e.g. parameter checking as in your foo_a() -example) and at the very end, where the regular return value is returned. Note that there can be multipe return statements in both areas, although it is uncommon for my functions, to have multiple "regular" exit points.

    int foo2(int param1, int param2) {
        if (!precondition1(param1)) return -1;//error
        if (!precondition2(param2)) return -2;//error
        if (param1==param2) return 0; // no error, but answer can be determined trivially       
    
        //computation
    
        return local_variable; //return regular result
    }
    

    here the first two return statements at the beginning might also be asserts or exceptions.

    If I do two different computations depending on the value of a flag or parameter (like food.isVegetable) I always use if-else and a single return statement after both as in the first example above.

    Also, if the level of intendation becomes too high, you might want to consider writing a separate function. This is not always possible but more often, than you might think. E.g. for erro checking, you can write a wrapper around the actual function that checkes for faulty input:

    int fooChecked(int param) {
        int retVal;
        if (param > 0 && param < 200) {
            retval = foo(param);
        } else {
            retVal = -1;            
        }
        return retVal;
    }