Writing Readable Code - Paul's Snippet Rewritten

Five brave people have contributed rewritten versions of Paul's code. Let's take a look at their approach.

I reformatted Paul's example using two spaces for every indent level (after then, begin, else, etc. ), hoping that this is the way he originally wrote it.

IMO, Paul's formatting (or my assumptions about his formatting) does not reflect the true code path, and trying to decipher the code paths become unnecessarily hard.

  try
    if ConditionA then
      Code(1)
    else if ConditionB then
      if ConditionC then begin
        if ConditionD then
          code(2);
        code(3);
      end
    else if ConditionE then begin
      code(4);
      morecode('X');
      if ConditionF then
        code(5)
      else if ConditionG then
        code(6);
    end else begin
      code(7);
      morecode('Y');
    end;
  finally
    code(8);
  end;

Here is a small exercize. If we say that the string "ABCDEFG" are all conditions TRUE, and the string "abcdefg" are all conditions false:
• What codes are run by "ABCDEFG"?
• What codes are run by "aBcdefg"?
• What string(s) would make code(6) run?

TS contributed a very nicely formatted version, which is effective in guiding us through the potential code paths. I like his style.

  try
    if ConditionA then
      Code(1)
    else if ConditionB then
      if ConditionC then
      begin
        if ConditionD then
          code(2);
        code(3);
      end // without the semi-colon
      else if ConditionE then
      begin
        code(4);
        morecode('X');
        if ConditionF then
          code(5)
        else if ConditionG then
          code(6);
      end
      else
      begin
        code(7);
        morecode('Y');
      end;
  finally
    code(8);
  end;

SS didn't quite manage to reflect the flow in the code and his formatting is similar to Paul's code.

  try
    if ConditionA then
      Code(1)
    else if ConditionB then
      if ConditionC then
      begin
        if ConditionD then
          code(2);
        code(3);
      end //; I assume this semi-colon has to go
    else if ConditionE then
    begin
      code(4);
      morecode('X');
      if ConditionF then
        code(5)
      else if ConditionG then
        code(6);
      end
    else
    begin
      code(7);
      morecode('Y');
    end;
  finally
    code(8);
  end;

Jolyon scores high on restructuring and simplifying, but he made one mistake in his change. Under which condition will his code behave differently form the original?

  procedure WhenC;
  begin
    if ConditionD then
      code(2);
    code(3);
  end;

  procedure WhenE;
  begin
    code(4);
    morecode('X');
    if ConditionF then
      code(5)
    else if ConditionG then
      code(6);
  end;

begin
  try
    if ConditionA then
      Code(1)
    else if NOT ConditionB then
      EXIT;

    if ConditionC then
      WhenC
    else if ConditionE then
      WhenE
    else
    begin
      code(7);
      morecode('Y');
    end;
  finally
    code(8);
  end;

AO goes even further than Jolyon in reformatting and comments: As I believe that proper formatting is not the only way to increase readability of code, I also refactored it a bit to reduce nesting where possible. If this were an actual code, I would have probably gone even further and introduced separate routines for the individual blocks, depending on their complexity.

  try
    { This comment explains why ConditionA is handled first. }
    if ConditionA then begin
      Code(1);
      Exit;
    end;
  
    { This comment explains why the aggregate of ConditionB and ConditionC is handled next. }
    if ConditionB and ConditionC then begin
      { This comment explains why ConditionD is relevant only in this block. }
      if ConditionD then
        code(2);
  
      code(3);
      Exit;
    end;
  
    { This comment explains why ConditionE is handled next. }
    if ConditionE then begin
      code(4);
      morecode('X');
  
      { This comment explains why ConditionF and ConditionG are relevant only in this block. }
      if ConditionF then
        code(5)
      else if ConditionG then
        code(6);
  
      Exit;
    end;
  
    { This comment explains the default handling. }
    code(7);
    morecode('Y');
  finally
    { This comment explains why the following code must execute no matter what. }
    code(8);
  end;

This is readable, but personally I think AO went a bit too far. Like Jolyon, he also missed a structural detail in the refactoring and broke the code. Which condition state(s) will cause the code to misbehave?

I am divided on the use of Exit. It can add clarity, but it can also be a big problem as you leave a lot of code "dangling". If you decide to move the exit - you have to be really careful to ensure that any states that suddenly move in or out of scope behave correctly. If there is a nesting church and a chunking church, I'm probably a "Nestorian".

I do agree that refactoring is a valuable tool to clarify and simplify, and we should make an effort to break down our code into manageable blocks, but in this particular case it probably isn't necessary.

Another thing: I avoid using {curly braces} for in-code commentary and use // instead. Why? Because if you need to comment out code containing select count(*) from table, those curlies will work where the (* comment *) fail. Should CodeGear add support for comment nesting? I don't know...

Anyways...
Here's how I would format the example. This is very similar to TS's example, except that I showel all the reserved words to the left side to leave the logic more visible and commentable, but I also add indentation to the innermost conditional code.

  try
    if ConditionA
     then Code(1)
     else if ConditionB
      then if ConditionC
       then begin
         if ConditionD
          then code(2);
         code(3);
       end
       else if ConditionE
        then begin
          code(4);
          morecode('X');
          if ConditionF
           then code(5)
           else if ConditionG
            then code(6);
        end
        else begin
          code(7);
          morecode('Y');
        end;
  finally
    code(8);
  end;

MJ adds a contribution with the following comment: "To be sure not to create any future bugs you should consider adding a begin end section after every if statement even if it is not required, but that will make the code more unreadable."

  try
    if ConditionA then
      Code(1)
    else if ConditionB then
    begin
      if ConditionC then
      begin
        if ConditionD then
          code(2);
        code(3);
      end
      else if ConditionE then
      begin
        code(4);
        morecode('X');
        if ConditionF then
          code(5)
        else if ConditionG then
          code(6);
      end
      else
      begin
        code(7);
        morecode('Y');
      end;
    end;
  finally
    code(8);
  end;

Good Points! In all honesty, I also screwed up the code blocks on first try. Conditional code will bite you if you are not very very careful. You should indeed think about what may happen if you need to add more code and/or conditions. Personally, I don't think a few more enclosures makes the code less readable. Here is how I would be more explicit in the use of enclosures to make the code less ambiguous.

  try
    if ConditionA
     then Code(1)
    else begin
      if ConditionB
      then begin
        if ConditionC
        then begin
          if ConditionD
           then code(2);
          code(3);
        end
        else begin
          if ConditionE
          then begin
            code(4);
            morecode('X');
            if ConditionF
             then code(5)
             else if ConditionG
              then code(6);
          end
          else begin
            code(7);
            morecode('Y');
          end;
        end;
      end;
    end;
  finally
    code(8);
  end;

There is no one true correct way of formatting.
The point I am trying to make is that code structure matter for understanding the code at first glance, and we should be mindful about how we lay it out. We should strive for consistency, but always keep clarity and unambiguity as priority one. Bend your rules, if you need to.

Paul, Thank you for creating such a devious code snippet!

P.S. If you haven't figured out the answers yet, load up your Delphi and run all the examples in StructureDemo.dpr.
No peeking until you have some suggestions! :)
(Hint: else starts a new block)

Edit: Added an example of using Exits instead of nesting to the initial post on formatting. It would be interesting to see DelphiFreak have a go at Paul's snippet.