Roman's Blog

Normally constants improve the readability and maintainability of your code. One example is the minimum value of a single. You can find it in the unit System.Math:

...
const
  MinSingle   =  1.4012984643248170709e-45;
...

Instead of writing the float value it is easier and more readable to use the MinSingle constant.
But today I found a way how to do this in a very bad way. Someone had to parse a text file. One feature was that is was possible to delete values. Therefore the writer of the file used a special value, a “*”. That’s why I found the following code:

const
  cStar = '*';
var
  sValue: string;
  ...
begin
  ...
  if sValue = cStar then
    DoDeleteValue
  ...
end;

This is complete nonsense! If you really have no idea how to name your constant then simply don’t use it! The name always should refer to the meaning of the constant and not to the content.
Maybe something like this is better:

const
  cDeleteValue = '*';
var
  sValue: string;
  ...
begin
  ...
  if sValue = cDeleteValue then
    DoDeleteValue
  ...
end;

During the last weeks it happened often that I had to write some command line apps. The reason for this is that more and more things are running automatically in tools like FinalBuilder (I will write an article about this later).

One thing that makes the command line app life easier is the encapsulation of the switches in a record (Current Delphi versions allow records with methods).

type
  TSwitches = record
  strict private const
    cHelpSwitch = '?';
    cBackupCountSwitch = 'c';
  public
    class function BackupCount: Integer; static;
    class function Help: Boolean; static;
    class procedure PrintHelp; static;
  end;

...

class function TSwitches.BackupCount: Integer;
var
  sCount: string;
begin
  FindCmdLineSwitch(cBackupCountSwitch, sCount);
  TryStrToInt(sCount, Result);
end;

class function TSwitches.Help: Boolean;
var
  sHelp: string;
begin
  Result := (ParamCount = 0) or (FindCmdLineSwitch(cHelpSwitch, sHelp));
end;

class procedure TSwitches.PrintHelp;
begin
  Writeln(Format(SUsageSample, [TDBSwitches.cBackupFolderSwitch, TDBSwitches.cBackupCountSwitch]));

  Writeln;
  Writeln(SOptions);
  Writeln;

  Writeln(Format(SHelpSwitch, [TDBSwitches.cHelpSwitch]));
  Writeln(Format(SBackupCountSwitch, [TDBSwitches.cBackupCountSwitch]));
end;

This record can be used directly in the .dpr file.

begin
  if TDBSwitches.Help then
    TDBSwitches.PrintHelp
  else
    TDBWork.DeleteBackup(TDBSwitches.BackupFolder, TDBSwitches.BackupCount);
end.

Today I found another common mistake. Someone used more than one object and has to free them.

var
  pObject1: TMyObject1;
  pObject2: TMyObject2;
begin
  pObject1 := TMyObject1.Create;
  try
    pObject1.DoSomething;
    pObject2 := TMyObject2.Create;
    try
      pObject2.DoSomethingElse;
    finally
      pObject2.Free;
    end;
  finally
    pObject1.Free;
  end;
end;

Instead of nested try…finally blocks it is easier and more readable to do the same with one try…finally block

var
  pObject1: TMyObject1;
  pObject2: TMyObject2;
begin
  pObject2 := nil;
  pObject1 := nil;
  try
    pObject1 := TMyObject1.Create;
    pObject1.DoSomething;
    pObject2 := TMyObject2.Create;
    pObject2.DoSomethingElse;
  finally
    pObject1.Free;
    pObject2.Free;
  end;
end;

This solution can still be improved. The first object can be created outside the try…finally block

var
  pObject1: TMyObject1;
  pObject2: TMyObject2;
begin
  pObject2 := nil;
  pObject1 := TMyObject1.Create;
  try
    pObject1.DoSomething;
    pObject2 := TMyObject2.Create;
    pObject2.DoSomethingElse;
  finally
    pObject1.Free;
    pObject2.Free;
  end;
end;

Please remember that all variables that are freed in the finally block must be initialized before the try statement.

I guess that most of you know this practice for many years but there are still people around who don’t.

Today I found some old source code. Someone wanted to write a file in the old OEM codepage. Therefore he wrote a small function that encapsulates the Windows API CharToOem function.

class function TStrUtil.ANSIToOEM(const ANSI: string): string;
var
  sBuffer: AnsiString;
begin
  if ANSI = '' then
    Exit('');

  SetLength(sBuffer, Length(ANSI));
  if CharToOem(PChar(ANSI), PAnsiChar(sBuffer)) then
    Result := string(sBuffer)
  else
    Result := ANSI;
end;

After that he added the OEM string to a TStringList and saved the TStringList to a file.

function TMyForm.SaveTheFile(const AFileName: string)
var
  pList: TStringList;
begin
  pList := TStringList.Create;
  try
    pList.Add(TStrUtils.ANSIToOEM('ÄÖÜ'));
    pList.SaveToFile(AFileName);
  finally
    pList.Free;
  end;
end;

I’m surprised that this code does work. Since Delphi 2010 strings are Unicode strings. This means that the OEM string is stored in a Unicode string variable.

Instead, I think is is much easier to use the wonderful TEncoding class.

function TMyForm.SaveTheFile(const AFileName: string)
var
  pList: TStringList;
  pOEMEncoding: TEncoding;
begin
  pOEMEncoding := nil;
  pList := TStringList.Create;
  try
    pOEMEncoding := TEncoding.GetEncoding(437);
    pList.Add('ÄÖÜ');
    pList.SaveToFile(AFileName, pOEMEncoding);
  finally
    pList.Free;
    pOEMEncoding.Free:
  end;