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:
- Formatting and Indenting
- Formatting on Syntax
- Naming
- 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..
- System
- Plattform
- Other RTL
- Frameworks like my FDK
- 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
tkStr : Log(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.
