Sunday, May 22, 2022

How to format your Delphi-Sourcecode!

At first:

Formatting your source code the Embarcadero way is not a mistake or to make it clear, probably the best way to format your code. Especially if you want to share your code with other developers.

But...This is not my way...

So I'm doing it wrong? No... I have good reasons to format my source code in a different way!



I have implemented some of my formatting rules many many years ago and some of them in the last 5 years. Some rules I developed at a time when nobody talked about that a procedure should have only 75 lines or "must" always fit completely on the screen. Therefore it was necessary to guess from the formatting what is "hidden" in the invisible part.

There are four kinds of rules:

  1. Formatting and Indenting
  2. Formatting on Syntax
  3. Naming
  4. Empty lines or other "small things"

All rules are "just" to make the code more readable or in some cases better maintainable.

One drawback of my rules: No formatter is able to format Delphi source code 100% according to my rules.

Therefore I started the development of my own code formatter some time ago. My formatter is using a different approach to format code. First, a tokenizer creates the syntax tree, and then a procedure is called for each part.

E.g. to format the uses list, a procedure "format_Uses" with all the necessary sources is called. By default, the code just formats it the Embarcadero way or you could implement your own method for this. So beyond some settings, you can do everything!

I'm very busy with my main "job" for some time and that's the reason this project is in the WIP folder.

So enough talk, here are my rules.

Let's start with a simple one... And don't expect a complete list here it would be out of scope for a little blog post. If you like my style of formatting or my rules, please write a comment. If enough developers would like to see more, perhaps I consider writing a complete rule book.

case Whatever of
  whNone : ;
end; // of case

All case ends gets this comment because this end is the only end without a begin.

TFoo = class   
  private
   
fWhatever : String;
   
fCount    : Integer; 

    function 
GetWhatever : String;
    procedure
SetWhatever( Const aValue : String );
    function  GetCount : Integer;
    procedure SetCount( aValue : Integer );
  public
    Constructor Create;
    Destructor  Destroy;override;

    Property Whatever : String  read GetWhatever write SetWhatever;
    Property Count    : Integer read GetCount    write SetCount;
end;

OK, this end has also not a begin, but the is not inside the source code where you have multiple levels of begin ends. For many years I've written FWhatever, but a lower f is more readable in many cases, like FField or fField. The lower "f" is easier to ignore while reading. Also, every function gets an extra space so that the method names are in the same column. the sane for the destructor. There is an empty line after the vars... And the properties got formatted by length for better readability.

Please compare this with:

TFoo = class   
private
  
FWhatever:string;
  
FCount:Integer; 
  function
 GetWhatever:String;
  procedure 
SetWhatever(const AValue:string);
  function GetCount:Integer;
  procedure SetCount(AValue:Integer);
public
  Constructor Create;
  Destructor Destroy;override;
  Property Whatever:String read GetWhatever write setWhatever;
  Property Count:integer read GetCount write SetCount;
end;

Well-formatted source code becomes more and more important for me the older I get.

Naming

Again field values get a lower "f", params get a lower "a", local vars in methods get a lower "l", and const values get a lower "c". I know the small l is not so easy to distinguish from the upper "I", for Interfaces.

But you would never write IFoo := NIL... 

User   
 
System.SysUtils
, System.Classes
, FMX.Graphics
, Web.HTTPApp
, FireDAC.Phys
, FireDAC.Phys.MySQL
// , FireDAC.Phys.SQLite
, Delphiprofi.FDK.FireDAC
, Delphiprofi.FDK.QRCode
, Delphiprofi.FDK.Server.ISAPIHandler
{$IFDEF DEBUG}
, Delphiprofi.FDK.Logging
{$ENDIF}
, Settings
, HTMLHandler
;

By formatting the uses with a leading "," and only one unit for each line, you can easily comment out some units and excluded unis by IFDEF is much better readable. After that, I like to sort my units..

  1. System
  2. Plattform
  3. Other RTL
  4. Frameworks like my FDK
  5. Units from the project. 

Please compare this with:

User   
  
Settings, Web.HTTPApp, System.SysUtils, Delphiprofi.FDK.FireDAC,
FMX.Graphics, {FireDAC.Phys.SQLite}, Delphiprofi.FDK.Server.ISAPIHandler, FireDAC.Phys, FireDAC.Phys.MySQL, System.Classes, Delphiprofi.FDK.FireDAC, Delphiprofi.FDK.QRCode, Delphiprofi.FDK.Server.ISAPIHandler{$IFDEF DEBUG}, Delphiprofi.FDK.Logging{$ENDIF}, HTMLHandler;

Formatter on Syntax

Do you remember these days, when your source looked like this:?



These days I created a simple rule...

Do you know, if the "if FileExists ..." has an else part? No, not from this point in the source code. Then you have to scroll down and if the procedure is very long, you have to scroll way too far down for this information.
If the "if" has an else part, the "then" is in the next line, if not the then is in the same line as the "if".

So simple, this if has an else:

if FileExists(fLogFilename) 
  then begin
         FS := TFileStream.Create(fLogFileName, fmOpenReadWrite);
...

This if has no else:

if FileExists(lLogFilename) then
  begin

    FS := TFileStream.Create(lLogFileName, fmOpenReadWrite);
...

Of course, never format it this way, because the lines from begin to end do not work:

if FileExists(LogFilename) then begin
  FS := TFileStream.Create(LogFileName, fmOpenReadWrite);
...

So it look like this if you have only one line:

if 
FileExists(aLogFilename)
  then FS := TFileStream.Create(aLogFileName, fmOpenReadWrite)
  else FS := TFileStream.Create(aLogFileName, fmCreate);

In any other cases, it looks like this:

if FileExists(cLogFilename)
  then begin
         FS := TFileStream.Create(cLogFileName, fmOpenReadWrite);
         Whatever := 'XX';
       end
  else begin
         FS := TFileStream.Create(cLogFileName, fmCreate);
         Whatever := 'YY';
       end;


and by the way... cLogFilename a const not a var anymore. And if you look up you can recognize one var is a (l) local, one is part of an object (f), and of is a param to this method (a) containing this code... If you just write LogFilename like in the bad example - you have no clue where the var is defined.

Empty lines and CR's


Where to put an empty line an where not, is the most overseen method to make your code more readable. Let's take a look at this bad example (stupid code):

Procedure TMainModel.LogVars(LogFilename:string);
var i:Integer;FS:TFilestream;
begin
 
LogFilename:=TPath.Combine(Path,Logfilename);startconvert:=true;
  if FileExists(LogFilename) then begin
    FS := TFileStream.Create(LogFileName, fmOpenReadWrite);
  end else FS := TFileStream.Create(cLogFileName, fmCreate);
  for i:=0 to varlist.count.1 do begin
    for var k:=0 to varlist.count-1 do varlist[k] := prepare(varlist[k]);
    
fStartConvert := true;
    if varlist[i].MustConvert then convert(varlist[i]);
    Case varlist[i].Kind of
      tkStrLog(FS,varlist[i].ForLog);
      tkInt : 
Log(FS,varlist[i].AsString);
    end;
    if varlist[i].MustConvert then reconvert(varlist[i]);
  end;
  startconvert:=false;
end;


{ -------------------------------------------------------------------------
  (c) by Peter Parker
  1998 Version 1.0 of Whatever...
  Procedure to display a message 
  ------------------------------------------------------------------------- }

Procedure TMainModel.Whatever(Display:string);
begin
  if Display.trim<>'' then MyMessage(Display) else
    raise Exception.Create('no Text to Display')
  if Display='-' then Memo1.Lines.Clear;
end;

Ok, now use my rules... before every "for" there is an empty line, also after the "for". The same rule applies to "if" and case, but not if before is a "begin" or "try" or after is an end ( of course ). Only one empty line between methods. Never write code behind an "elseless" then. (if you read the Display trim stuff... at the first millisecond it looks like this "if" raises the exception.  Here is my code (and simple i is kept, and no stupid comments) :


Procedure TMainModel.LogVars( Const aLogFilename : String );
Var 
  i            : Integer;
  lFS          : TFilestream;
  lLogFilename : String;
begin
  
lLogFilename  := TPath.Combine(cPath, aLogfilename);

  if FileExists(lLogFilename) 
    then lFS := TFileStream.Create(cLogFileName, fmOpenReadWrite)
    else lFS := TFileStream.Create(cLogFileName, fmCreate);

  try
    for var k := 0 to varlist.count - 1 do
      
varlist[k] := prepare(varlist[k]);
    
    for
 i := 0 to varlist.count - 1 do
      begin
        fStartConvert := true;

        if varlist[i].MustConvert then 
          convert(varlist[i]);

        Case varlist[i].Kind of
          tkStr : Log(FS,varlist[i].ForLog);
          tkInt : 
Log(FS,varlist[i].AsString);
          
{$IFDEF DEBUG}
          else raise DeveloperException.Create('you forgot a case entry for .Kind');
          {$ENDIF}
        end; // of case
  
        if varlist[i].MustConvert then 
          reconvert(varlist[i]);
      end;
  finally
    lFS.Free;
  end;

  if fStartConvert then
    fStartConvert := false;
end;

Procedure TMainModel.Whatever(Display:String);
begin
  if Display.trim = '' then
    raise Exception.Create('no Text to Display'); 

  MyMessage(Display);
end;

Every case that has a limited set will raise an exception so you "never" forget to update your cases. If possible Early exit a procedure - this rule for (exit and exceptions). Strings as params get a "Const"... Sometimes you need a local var because of "const", but better if you copy it to a local instance at the very end than passing strings around without a "const". (Consider the string has passed around more than one method.

Now I hope you have an idea why I do it my way. If you consider that my rules are something you would like to use in your code... Be my guest and please leave a comment.