Code Indentation Density

I once worked in a VB6 shop where we had one developer insist that a tab in the code editor was 3 spaces and not 5 like the rest of us were using.  It was no big deal until someone else had to work on his code.  As it turned out, the developer left after a year and we had to extend everything he ever worked on in addition to the inevitable bug fixes.  I think we generated enough hatred for that one developer to last a lifetime because not only did we have to fix his buggy code but we had to reformat it too.

The Real Studio code editor handles indentation for you and you have no control over it.  I know some hate that but I’m okay with it.  It enforces consistency across source code so that it all reads the same.  As someone who reads and edits a lot of Other Peoples Code (OPC) I can appreciate that.

The Real Studio IDE puts in visual indicators (lines) for loops.  This is convenient but I’ve found that once I get past three layers of nested If-Then or For-Next or While loops it gets harder to read.  Perhaps that just me but I suspect that others have the same issue.

Take this first (totally contrived and nonensensical) example:

Real StudioScreenSnapz001

 

It’s not an exceptionally hard method to figure out.  First we check that our array has content.  Then if we have only one item we do something and if we have more than one we iterate through them and do some processing and finally return true.

Towards the bottom of the method where we’re using the For Next loop to process the items I find myself having to highlight the loop so I can visually keep it straight in my mind.  I decided to rewrite it a bit to the following:

Real StudioScreenSnapz002

The only difference is that I immediately bail and return from the method if I have nothing in my array.  It cuts down on my indention nesting by one level ands thus makes it easier to read (in my opinion).

This got me thinking about what else I could do to reduce the indent density.  So I tried this:

Real StudioScreenSnapz003

Switching from the If-Then-Else structure to a Select-Case makes it less dense.  I don’t know about you but I generally don’t think about a Select-Case statement when dealing with array processing.  Perhaps that is because I don’t generally care if I have only one item in the array or not as in this case.

In some situations there’s simply no way around huge complex nested loops and if-then statements.  What I tend to do is take my innermost loops and convert then into their own functions and name them something that makes total sense (to me at least).  If that means a function is named “These_Are_Not_The_Droids_You_Are_Looking_For” that’s okay.  It eliminates one or more levels of nested code.

Regardless, making your code easier to read should be a goal in life.  Making code less dense and easier to understand will aid that process.  Of course comments can help but to each their own on that.  White space can help too.  Remember:  you’re not coding just for now or five weeks from now but for five years from now.

I hope everyone has had a good holiday season so far.  I’ve been enjoying my time off by —- coding, of course!

 

13 thoughts on “Code Indentation Density

  1. Coming from other fast compilers, I wonder how expensive the creation of types and objects are. Each time I type a Dim, I have fears I am loosing CPU Core precious cycles (as we have just use 1 core for the whole program).

    You can condense just a bit more like:

    “select case aro.ubound” instead of creating ICount unless you are going to debug it (tsk, tsk… debugger).

    In the loop, I don’t really know what is the price for creating the object p for the FTParagraph class. Does it only pass just a pointer for the actual instance of the array or is there more involved? Have you noticed is it CPU intensive (for a huge loop)?

    if aro(i).IsSpellingDirty and ProcessDirty aro(i) would work, but yeah, makes code ugly as the VB example.

    Readibilty vs Performance would be a very very nice blog!

    Happy new 2013!

  2. @Amando Blasco
    I actually prefer to use aro.ubound inline rather than create a variable. But I know others that do not like it at all so I opted for the more verbose (to me) methodology. We had a discussion on whether the inline method was more ‘readable’ in the past couple of months.

    I *believe* when you create a local variable for the object p it’s just passing the pointer. It should be pretty fast and efficient as you’re not creating a new object just creating a new reference to it.

  3. @Bob Keeney
    Me too, I try to avoid creating unnecessary vars and objects. Maybe Joe Ranieri can gives as some hints on the reference counter or kind of GC RS uses for freeing instances.

    Well, creating a new instance adds more overhead as the constructor has to be called, etc.. but creating an object might (in theory) create more than a simple variable (references, etc). Again it’s something I always wondered, what is the most effective way to treat objects.

    This example is maybe not the best to describe, but sometimes we need to create several object pointers for readability sake in a loop.

    Please, let me take this thread to drop a question that is driving me insane. I already have posted to forums, but never got an answer (dunno if it’s too obvious or too difficult). It’s somewhat related to this thread. Sometimes I need to “clone” an instance (let’s say we have array of XML or JSON objects parsed and stored to RS properties). Using p.whatever (setter, or method) will change the referenced array, and sometimes I need to get a copy of the instance and use the array as a container of processed objects.

    Has anyone know a way to clone an instance without having to create a clone method that you manually have to set all the class properties to avoid this?

    Sub Clone() {

    Dim m as new class
    m.property = me.property
    return(m)
    }

    We a few proprieties is ok, but for big classes drives to make tons of mistakes if you don’t keep the clone method updated.

    Sorry for stealing your answer Bob!

  4. I’ll just point out that you have the “Cyclomatic Complexity Report” in RB Code Reports to point out where your code may need possible refactoring.

  5. The visual indicators (lines) for loops are very helpful but as you pointed out it can get confusing when there are too many layers of lines. I think it would be better if the visual indicator lines would have different colors, for example: layer 1 = lightgrey, layer 2 = lightred, layer 3 = lightblue, layer 4 = lightgreen …

  6. I’d isolate the “If something wrong/false/0”-Test entirely from the rest. The Select Case variation looks different, a matter of taste, but you still have the code nested 2 levels deep. In your second example, if you replace the first “else” with “return false (and next line): end” then the for i loop comes to the top level.

  7. if icount = -1 then return false
    if icount = 0 and aro(0).isDirty = false then return
    for i as integer = 0 to iCount
    dim p as FTParagraph = aro(i)
    if p.isSpellingDirty then
    ProcessDirty p
    else
    ProcessClean p
    end if
    next
    return true

  8. The code indentation density, as you call it, is a good subject to discuss, however I’m not in favor of the solution. An age-old recommendation for procedures/methods/subroutines is to have a single entry point and a single exit point. Putting one or more returns in the middle of procedure is not really the desirable solution. First, I’d suggest making the loop handle all cases regardless of how many times it loops. There’s nothing wrong with using it even when there is only one element in the array. Second, the creation of other, smaller, and more atomic procedures usually helps. I will admit to sometimes putting a return in the code before the end of the procedure – I’m not claiming to be perfect – but, in general, it should be avoided. BTW . . . a tab equivalent to three spaces is the exact perfect amount! 😉

  9. “An age-old recommendation for procedures/methods/subroutines is to have a single entry point and a single exit point. ”

    I’m a great believer in this rule ever since one dark day many years ago when clean-up code was bypassed and a database transaction stayed open for rather a long time

  10. @Russell
    One entry one exit leads to convoluted code that has unclear paths if there’s any complex paths in the body of the method

    I like to fail as early as possible & skip all the nested if’s entirely where possible

    Heck, if you’re religious about “one entry one exit” stick a
    goto EndOfMethod
    and just jump to the end of the method just to return

    Fail Early. Fail Often. Fail Fast.

    Failure to do so leads to situations where your software stumbles along and eventually dies an inglorious death and you get a bug report that you ponder over & wonder “Now how the heck did blarfsnart get to be nil?” and you end up chasing side effects rather than root causes.

    Heck have a read of the sample code on and just look at the code that has returns inline vs the single entry single exit code.

    Exceptions, if you raise them, dont fit this one entry one exit model.
    And if coroutines came along they dont either.

    Also I agree that methods in general should be as short or compact as possible and no longer (or shorter)

Comments are closed.