Unsure tabbing with Pascal [closed]

Question

Hi I'm trying to tab some code, please could someone check to make sure I have done this correctly (Changed some code for simplicity):

begin
  if Password <> Database['Database']
    then showmessage ('Message')
    else
  if NewPassword <> Retype
    then showmessage ('Message')
    else
      begin
        if Message (yes, No, etc) =yes
          then
            begin
              List
              List
              List.post;
              showmessage ('Message')
            end
          else close;
      end;
end;

Answer 1

As noted in the other answers, style is highly subjective, and for that reason it is difficult to give an objective answer. However, something that can be said is how your code should look according to Embarcadero's Object Pascal Style Guide, and that is what I will attempt to answer (with some more detail than you asked for). When the same issue occurs in multiple locations, I will only mention it once.

begin
  if Password <> Database['Database']
    then showmessage ('Message')

Here, the casing of showmessage should be ShowMessage, which is also the name given to the procedure in the Dialogs unit. There should not be a space between ShowMessage and (.

Your use of then on the next line is unusual but okay, and you're right to indent it in that case.

    else
  if NewPassword <> Retype

This second if statement is part of the else branch of the first if statement, and should be indented accordingly, possibly by placing it on the same line as the else.

    then showmessage ('Message')
    else
      begin

begin and end should not have extra indentation: the substatements should be two spaces to the right of the else above.

        if Message (yes, No, etc) =yes

The fact that you have a space before the =, but not one after it, is not something I've seen before, but this is not a situation where the style guide expresses any preference, so that is okay.

          then
            begin
              List
              List
              List.post;
              showmessage ('Message')
            end
          else close;

This deserves an extra mention: else followed by a statement is explicitly called out as having fallen out of favour, but nonetheless okay, in the style guide.

      end;
end;

Something else worth mentioning is that your use of two spaces for indentation is exactly what the style guide says to use.

Answer 2

Personally, I'm an else if guy and also I'm against hanging then:

procedure outer;
begin {outer's}
    if 2 * 2 = 4 then
    begin {this belongs to if...then level}
        Writeln('make bools');
    end
    else if Sin(X) = 3 then
    begin {and this too}
        Writeln('not war');
    end;
    else if True and SoOn then
        {...}
end; {/outer's}

But in your case I'd better avoid large compound statements and heavy nesting in else branch:

begin
    if not CurrentPasswordIsCorrect then
    begin
        Notify('bad pass');
        Exit;
     end;

     if NewPasswordEntry1 <> NewPasswordEntry2 then
     begin
        Notify('you forgot your password already');
        Exit;
     end;

     { and so on }
end;

Sometimes the rule about writing normal flow code to then branch and abnormal to else branch becomes a false friend and inviting writing a lot of nested statements to else which in turn only hurts readability instead of improving it.

Answer 3

This is a coding style question, and may not survive here very long. :-) (Coding style is very much a matter of personal opinion, and there are an awful lot of different ones out there.) I'll give it a shot, anyway. :-)

I'd do it slightly differently. IMO, this clearly shows the proper pairings of if..else and begin..end to a new programmer:

begin
  if Password <> Database['Database'] then
    showmessage ('Message')
  else 
    if NewPassword <> Retype then
      showmessage ('Message')
    else
    begin
      if Message (yes, No, etc) = yes then
      begin
        List;
        List;
        List.post;
        showmessage ('Message');
      end
      else
        close;
    end;
end;

In my own code, I'd do it a little differently still (but only a minor difference). I'd move the else if password to the same line (it reduces one level of indent, and to me makes the flow of the code more clear. We have three possible options, and there are three options clearly shown (if this, else if this, else this):

begin
  if Password <> Database['Database'] then    // option 1
    showmessage ('Message')
  else if NewPassword <> Retype then          // option 2
    showmessage ('Message')
  else                                        // option 3
  begin
    if Message (yes, No, etc) = yes then
    begin
      List;
      List;
      List.post;
      showmessage ('Message');
    end
    else
      close;
  end;
end;

There are only a couple of other code areas where formatting sometimes makes a difference. I'll try to quickly touch as many of them as I can think of off-hand.

Case statements:

case i of
  0: DoThingForZero;            // Only one line to execute for 0
  1: begin                      // Two things to do for 1
       DoSetupForOne;
       DoThingForOne;
     end;
  2: DoThingForTwo;
else                            // Handle anything other than 0, 1, 2
  DoThingsForOtherValues;
end;

While statements:

while not Query1.Eof do
begin
  // Process each field in current record of table
  Query1.Next;  // Move to next row (easy to forget, infinite loop happens. :-)
end;

Repeat statements:

i := 1;
repeat
  i := i + SomeFunctionResultReturningVariousValues();
until (i >  50)

For loops:

for i := 0 to List.Count - 1 do
begin
  ProcessItem(List[i]);
end;

for i := List.Count - 1 downto 0 do
  List[i].Delete;

For..in loops:

for ch in SomeString do           // For each character in a string,
  WriteLn(ch, ' = ', Ord(ch));    // write the ordinal (numeric) value 
ReadLn;

Try..finally:

SL := TStringList.Create;        // Create object/open file/whatever (resource)
try
  // Code using resource 
finally
  SL.Free;                       // Free the resource
end;

Try..except:

try
  // Do something that might raise an exception
except
  on E: ESomeVerySpecificException do
  begin
     // Handle very specific exception 
  end;
  on E: ESomeLessSpecificException do
  begin
    // Handle less specific exception
  end;
  else
    raise;
end;

Try..finally with try..except:

SL := TStringList.Create;         // Allocate resource
try
  try
    // Do something that might raise exception
  except
    // Handle exception as above
  end;
finally
  SL.Free;                       // Free resource
end;

Answer 4

Here's how I would format that code:

begin
  if Password <> Database['Database'] then
    showmessage ('Message')
  else
  if NewPassword <> Retype then
    showmessage ('Message')
  else
  begin
    if Message (yes, No, etc) =yes then
    begin
      List
      List
      List.post;
      showmessage ('Message')
    end
    else
      close;
  end;
end;

The main differences are:

• Keeping the then at the end of the same line that if is on (unless there are multiple conditionals).
• Keeping else statements in line with if statements