15

I hear a lot that you should not write functions larger than one screen size, that you should extract things into functions if you call things very often and all these coding guidelines. One of them is also keep your indentation level low. Some people told me even I should max indent 4 times.

But in every day programming it keeps happening that I need O(n^2) or O(n^3) loops with some flow control, which creates out of nowhere 7 or 8 layers of indentation...

Are there any good ways to avoid that? One problem is, if you try to avoid complex if/else statement nesting I tend to do this in one row which can look very terrible and be unreadable and not very easy to maintain.

3
  • the "loops" part of this (somewhat too broad) question is asked and answered in Declaring functions in order to avoid explicit nested loops
    – gnat
    Dec 6, 2013 at 11:29
  • some of that control flow can probable benefit from break and continue to lessen some indentation Dec 6, 2013 at 11:37
  • They're guidelines, not rules - don't get too dogmatic. The idea is to keep things readable and maintainable. Generally, splitting things up & keeping them short helps this. If you find that splitting things up makes it harder to follow, don't do it. Dec 6, 2013 at 20:53

5 Answers 5

20

Inverted If

Most developers think in terms of nested logic. It's difficult for them to invert their logic to reduce nesting. Deeply nested code is more difficult to read, and to reduce nesting you often have to invert your logic.

When a programmer writes code they write in a logical tree structure like this.

 bool function publish(Document doc)
 {
    if(doc != null)
    {
        if(doc.status == "finished")
        {
            doc.print();
            if(doc.locked)
            {
                doc.unlock();
            }

            return true;
        }

        return false;
    }

    return false;
 }

The above is logically correct, but it introduces indents that make it difficult to maintain in the future. It's difficult, because additional logic will require deeper blocks. The deeper it gets the more likely a bug will be introduced.

By inverting your if statements and combining them you can reduce the nesting.

 bool function publish(Document doc)
 {
    if(doc == null || doc.status != "finished")
    {
        return false
    }

    doc.print();

    if(!doc.locked)
    {
        return true;
    }

    doc.unlock();

    return true;
 }

This inverted kind of logic keeps all the function operations within the first indented block and if statements are only used to terminate the function.

Split Loop And Work

Don't perform your work operations inside a loop block. Separate iteration from the task itself. This creates functions of single purpose.

Take this example

function void PrintDocuments(List<Document> pDocuments)
{
    for(int doc=0; doc < pDocuments.Count; doc++)
    {
        for(int page=0; page < pDocuments[doc].Pages.Count; page++)
        {
            pDocuments[doc].Pages[page].Print();
        }
    }
}

While that isn't very complex it does create 3 levels of nesting, and the function is performing three different tasks.

Split the work and loops makes it easier to maintain.

function void PrintDocuments(List<Document> pDocuments)
{
    for(int doc=0; doc < pDocuments.Count; doc++)
    {
        PrintPages(pDocuments[doc].Pages);
    }
}

private function void PrintPages(Document pDocument)
{
    for(int page=0; page < pDocument.Pages.Count; page++)
    {
        PrintPage(Pages[page]);
    }
}

private function void PrintPage(Page pPage)
{
    pPage.Print();
}

One could argue that the above is extra work, but the time spend to split the tasks and create functions of single purpose will reduce effort later when maintaining the code. The reduced indention makes it easier to read. The work to be completed by a loop is never performed inside a loop, but is handled by a dedicated function for that work PrintPage.

Conclusion

Single purpose and inverted logic are the keys to reducing indentations.

5
  • 2
    Your code is unnecessarily complex: if (!doc.locked) { return true; } doc.unlock(); return true; should be simply: if (doc.locked) { doc.unlock(); } return true; Also, what is the point of wrapping page.print() with printPage(page) ? Dec 7, 2013 at 0:11
  • 8
    @kevincline I'm trying to illustrate a point. While my examples might be overkill in reducing indentation. It answers the OP's question of how do I reduce indention?. My code is simply longer but not more complex. You just perceive it as complex because your mind is used to thinking of if statements as logical blocks. I don't write code to this extent, but it's an example of how to remove all indents. The purest way of doing it is to keep all operations on the first indent, and not to do any work inside blocks. That's why they only contain return statements.
    – Reactgular
    Dec 7, 2013 at 1:43
  • 1
    @kevincline The point of the PrintPage function was to only illustrate the concept of single purpose functions, and removing all operations from inside loops. Yes, one line of code in a function is not a good example but coming up with good examples all the time is hard ;).
    – Reactgular
    Dec 7, 2013 at 1:46
  • 1
    The first rewritten code is actually wrong. It returns false when a document is not finished but the initial implementation returns true instead.
    – Andy
    Jul 25, 2017 at 11:57
  • @DavidPacker I didn't create the example, obviously, but if I had to guess it was the nested version that was 'wrong'. I think this is exactly why nesting is bad. It's actually much harder to follow because what is returned as a result of the guard condition is separated from the condition.
    – JimmyJames
    Jul 26, 2017 at 16:48
16

As you have already noticed, bunching up control logic to reduce indentation is totally counter-productive. The problem is not actually with levels of indentation, or number of lines. Those things are just indicators of the actual problem: doing too much in one piece of code.

The solution is to refactor nested, complicated loops into separate methods so that both the calling and the called method can be understood as doing one sensible thing. If your loops need too many local variables to be easily separable, a method object can be useful which encapsulates those variables with a large local scope into the class scope of a new special-purpose mini-class.

The same applies to complicated controller logic: if the condition of continuing or stopping a loop contains many complicated expressions, then refactor it into a boolean method with a sensible name. The object is to make the top-level logic to be obviously correct provided that the ancillary functions are correct, and the helper functions so small that they can be verified easily by themselves.

6

The idea behind these guidelines is that, when you find yourself in a situation such as yours, that there is most likely (not always, mind you, but still more often than you think) something wrong with your algorithm, and you should step back and rethink it.

I actually don’t think that refactoring will help you much, if it’s indeed an O(n²) or O(n³) algorithm – it might even worsen legibility (for people with wide enough terminals…) and reduce speed. (Of course, if you can improve the situation by refactoring without losing anything, by all means do so!) Really, step back, maybe let a coworker or someone else look at your code and algorithm and give advice… and if nothing else helps, ignore the guideline for that one function – with a detailed comment on top, of course.

1
  • 3
    +1 Absolutely true. If you can make it work, improving your algorithm is incomparably better than any amount of code cleanup could ever be. Dec 6, 2013 at 11:41
3
  • Put the innermost loops into functions. This also aids readability.
  • Rearrange your code to short circuit instead of nest. For example, instead of if (condition) {big long nested code}, use if (!condition) return;
  • Preprocess data into an intermediate form that doesn't require as much nesting. For example, if you copy a linked list into an array at the start, you might be able to avoid a nested loop later that traverses the linked list.
  • Use recursion instead of nested loops. Recursion often greatly simplifies deeply nested code.
  • Use exceptions to exit nesting during error conditions instead of checking at every level.
  • Try to find algorithms that are more efficient than O(n3).
1

Are there any good ways to avoid that? One problem is, if you try to avoid complex if/else statement nesting I tend to do this in one row which can look very terrible and be unreadable and not very easy to maintain.

Use local functions to embody the bodies of these conditionals. By doing this you cut a line between the workflow (supervisor) and the tranformations (performers), which improves clarity.

At a first glance, adding local functions makes reading harder because it breaks the reading flow of the program, but actually, it improves clarity because it separates the workflow from the transformations.

1
  • With good names, adding local functions can actually improve readability. At least, I find it easier to understand isReadyToGo(thing) than (thing.hasA() && thing.canB() && !thing.isC() && ...).
    – TMN
    Dec 6, 2013 at 21:04

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