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.

6 comments:

  1. Oh behave. Nearly ALL of your formatting makes me sick. Of course "you have a reason" for doing something different than the rest. Why can't you stick to the "official" source code formatting done be Borland/CodeGear/Embarcadero? I ALWAYS have to argue with some collegues about "their superior" code format style - what you don't understand is that some other who needs to read your code loses a lot of time "reformatting" your code (even it's done only inside their brain). That's totally wasted time just because you think your style is super cool.
    And of course you don't even stick to your own rules in the code above (which makes me even more sick), like, is there a space before a ":" and an opening "(" or not... see: "Whatever(Display:string)" and "( Const aLogFilename : String )"

    Your incoherent code style is just disgusting. The main rule while working in a team is to stick to ONE code formatting guide (which clearly states your a lone warrior). And in order to avoid loooong discussions why this or that is better, why not just use the official one. Which helps even yourself reading the official VCL code! Incoherent source code formatting is one of the big Delphi fails for beginners, just take a look at StackOverflow or somewhere - you'll find 1000 different styles, which really doesn't help beginners at all writing clean code right from the start.
    Sorry to be that rude, but it's an ongoing everyday struggle with different delphi developers. Please just stick with the official code style, else the others can't easily read your code. And I'm quite sure you'll get used to that in a short amount of time (as everybody does) and then it feels the same way natural than your now own code style.
    (That's one thing I like about Python: if you don't stick to a coherent indentation your code does not run).
    In the interest of all sane Delphi developers, please don't create any more "superior" code formatting style guide, thanks.

    ReplyDelete
    Replies
    1. Thank you for your blunt opinion. Source code, just like language development, is constantly changing. Many UCSD Pascal programmers have turned over in their graves because variable declarations or procedures suddenly appear in the middle of the code. The same applies to the source code formatting. Fortunately I don't care if someone doesn't like my formatting. Many of my rules make sure that I can read my code faster and better, even if I look at a method again after 5 years... My time is too valuable to get into a discussion here... I do it the way I want.

      Delete
    2. I really think you are a great Delphi programmer overall - I read about your FDK/other projects all the time and often said to me, hey that sounds cool, let's see how he is solving some problems there - but as soon as I saw some of your code, I immediately closed the project, just because it is hard to read *for any other than yourself*. Of course you don't have to care about others while coding your own projects, but if they have a hard time reading your code, they may just go away instead of bringing a project further along. And no, I'm used to the official Delphi code style and your code is NOT readable to me, despite your spacing effort - it's just does not "fit".

      Sure, UCSD and all other variants didn't had a specific style (what you've learned in a course or in a book was sometimes totally different styled in another book). But this is not UCSD here, it's Embarcadero Delphi. And they DO have a code style. I may sound very harsh - and I appreciate publishing my comment after all ;-) but you totally sound like some of my collegues that despite having the Embarcdero style guide set in stone with all of our projects (with 2 small changes), they always argue about their superior reading code style, which is a nightmare for all others.
      And you're quite right with "Well-formatted source code becomes more and more important for me the older I get" - you began to realize while reading your old cumbersome formatted code - which is now hard to read even for you - that a consistent code style helps a lot. Yes, but no, don't create a new cumbersome code style, which isn't well readable in some years by yourself as well...

      You see this is a very controversy topic here, as one styling is better readable to one and another is better readable to another one. There won't be a formatting that is suitable to all coders, but fortunately there is an offical one, so there shouldn't be no need to create a completely new one.

      Btw: "Be my guest and please leave a comment" and "My time is too valuable to get into a discussion here" does not really belong together, isn't it?

      Delete
    3. Leave a comment means:
      a.) I like you code formatting...Please provide a full list...
      b.) I dislike you code formatting. I stay with the Borland way...

      Btw. I have no fear to leave other opinion on my blog, but I've established my coding style and improved it,
      and I'm doing this for 40 years now...
      If you like my FDK or my D.MVVM you can just press CRTL-D to reformat it if you have bought it.

      Delete
    4. tldr;
      b) I dislike your code formatting, please stay with the Borland way ;-)

      Delete
  2. Love it. I think the obvious answer to "why" is that structurally formatted code is peaceful to read and deal with. Other code looks like a road crash to me.

    I'll never understand why other people seem to be happy not drawing out the symmetries that are hidden in code written like ...
    FWhatever:string;
    FCount:Integer;
    function GetWhatever:String;
    procedure SetWhatever(const AValue:string);
    At least the IDE can sometimes line up the colons, I suppose.

    I guess "normal" formatting might be a side effect of time pressures
    But yes, I want alignment and symmetry as much as possible.

    ReplyDelete