[CleanCode] Beispielklasse TDataLocation

[neo4a]

Eine frühere Diskussion über Delphi-Spring führte über CleanCode-Design zu Marx-Zitaten und dem Wunsch zu mehr Sachdienlichkeit. Ich stelle hier eine 200-Zeilen-Klasse zur Diskussion, die ein wohl häufig auftretendes Problem angeht: Wo sucht und speichert eine App ihre Daten?

Einmal abgesehen vom Nutzwert (portable App, CommandLine-Parameter, ConfigFile) geht es mir darum, praktisches Delphi-CleanCode-Design zu diskutieren und bei Interesse weiterführend noch die Einbindung mittels Spring-DI zu zeigen.

Die Klasse TDataLocation selbst veröffentlicht nichts als den Konstruktor. Alle Schnittstellen werden über die Interface-Deklaration (hier Fluent-Interface anstatt Properties mit Getter/Setter) veröffentlicht.

Im Ausgangs-Design muss die Unit uDataLocation zwar noch dort eingebunden werden, wo die Funktionalität benötigt wird. Mittels Spring-DI wird mit geringer Anpassung dann eine völlige Entkopplung möglich sein. Insgesamt fehlt zwar noch etwas Funktionalität und die Klasse bindet auch noch eine Hilfs-Unit (uUtils für GetCmdLineSwitchValue() und GetSpecialFolder()) ein, das habe ich aber insbesondere in Kauf genommen, um das 200-Zeilen-Limit einhalten zu können.

Delphi-Quellcode:

unit uDataLocation;

interface

type
  IDataLocation = interface
    ['{72C10C78-A39E-4005-8D53-57FD44A9B7F4}']
    function ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean;
    function DataFileExt(aDataFileExt : string) : IDataLocation;
    function DataCmdParam(aDataCmdParam : string) : IDataLocation;
    function ConfigFileExt(aConfigFileExt : string) : IDataLocation;
    function ConfigCmdParam(aConfigCmdParam : string) : IDataLocation;
    function ConfigSection(aConfigSection : string) : IDataLocation;
    function DataFileKey(aDataFileKey : string) : IDataLocation;
  end;

type
  TDataLocation = class(TInterfacedObject, IDataLocation)
  private
    FAppName        : string;
    FDataFile       : string;
    FConfigFile     : string;
    FDataFileExt    : string;
    FDataCmdParam   : string;
    FConfigFileExt  : string;
    FConfigCmdParam : string;
    FConfigSection  : string;
    FDataKey        : string;
    FIsLocal        : Boolean;
    FIsUSB          : Boolean;

    function FindDataCmd : Boolean;
    function FindConfigCmd : Boolean;
    function FindLocalConfig : Boolean;
    function FindLocalFile : Boolean;
    function FindUserDataFile : Boolean;
  private // IdcDataLocation
    function ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean;
    function DataFileExt(aDataFileExt : string) : IDataLocation;
    function DataCmdParam(aDataCmdParam : string) : IDataLocation;
    function ConfigFileExt(aConfigFileExt : string) : IDataLocation;
    function ConfigCmdParam(aConfigCmdParam : string) : IDataLocation;
    function ConfigSection(aConfigSection : string) : IDataLocation;
    function DataFileKey(aDataFileKey : string) : IDataLocation;
  public
    constructor Create;
  end;

implementation

uses
  SysUtils, Forms, IniFiles,  ShlObj,
  uUtils;

const
  cDataFileExt   = '.zip';
  cDataCmdParam  = 'data';
  cConfigFileExt = '.ini';
  cConfigCmd     = 'config';
  cConfigSection = 'Config';
  cDataFileKey   = 'Data';

function TDataLocation.ConfigCmdParam(aConfigCmdParam: string): IDataLocation;
begin
  Result          := Self;
  FConfigCmdParam := aConfigCmdParam;
end;

function TDataLocation.ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean;
begin
   Result := True;
   if aDefaultFileName > '' then
     FDataFile := aDefaultFileName;
   try
     If FindDataCmd then exit;
     If FindConfigCmd then exit;
     If FindLocalConfig then exit;
     If FindLocalFile then exit;
     If FindUserDataFile then exit;
     Result := False;
  finally
     aConfigFileName := FDataFile;
  end;
end;

function TDataLocation.ConfigFileExt(aConfigFileExt: string): IDataLocation;
begin
  Result         := Self;
  FConfigFileExt := aConfigFileExt;
  FConfigFileExt := ChangeFileExt(FAppName, FConfigFileExt);
end;

function TDataLocation.ConfigSection(aConfigSection: string): IDataLocation;
begin
  Result         := Self;
  FConfigSection := aConfigSection;
end;

constructor TDataLocation.Create;
begin
  FAppName        := Application.ExeName;
  FDataFileExt    := cDataFileExt;
  FDataCmdParam   := cDataCmdParam;
  FConfigFileExt  := cConfigFileExt;
  FConfigCmdParam := cConfigCmd;
  FConfigSection  := cConfigSection;
  FDataKey        := cDataFileKey;
  FIsUSB          := False;
  FIsLocal        := True;
  FDataFile       := ChangeFileExt(FAppName, FDataFileExt);
  FConfigFile     := ChangeFileExt(FAppName, FConfigFileExt);
end;

function TDataLocation.DataCmdParam(aDataCmdParam: string): IDataLocation;
begin
  Result        := Self;
  FDataCmdParam := aDataCmdParam;
end;

function TDataLocation.DataFileExt(aDataFileExt : string): IDataLocation;
begin
  Result       := Self;
  FDataFileExt := aDataFileExt;
  FDataFile    := ChangeFileExt(FAppName, FDataFileExt);
end;

function TDataLocation.DataFileKey(aDataFileKey: string): IDataLocation;
begin
  Result   := Self;
  FDataKey := aDataFileKey;
end;

function TDataLocation.FindConfigCmd: Boolean;
var
  aFileName : string;
begin
  Result := False;
  if GetCmdLineSwitchValue(aFileName, FConfigCmdParam) then
  begin
    FConfigFile := aFileName;
    Result      := FindLocalConfig;
  end;
end;

function TDataLocation.FindDataCmd: Boolean;
var
  aFileName : string;
begin
  Result := False;
  if GetCmdLineSwitchValue(aFileName, FDataCmdParam) then
  begin
    FDataFile := aFileName;
    Result    := FindLocalFile;
  end;
end;

function TDataLocation.FindLocalConfig: Boolean;
var
  aIniFile : TIniFile;
begin
  Result := False;
  if FileExists(FConfigFile) then
  begin
    aIniFile := TIniFile.Create(FConfigFile);
    try
      FDataFile := aIniFile.ReadString(FConfigSection, FDataKey, FDataFile);
      Result    := FindLocalFile;
    finally
      aIniFile.Free;
    end;
  end;
end;

function TDataLocation.FindLocalFile: Boolean;
begin
  Result := False;
  if FIsLocal then
    Result := FileExists(FDataFile);
end;

function TDataLocation.FindUserDataFile: Boolean;
var
  aDataPath : string;
  aFileName : string;
begin
  Result := False;
  if not FIsUSB then
  begin
    aFileName := ExtractFileName(FDataFile);
    aDataPath := IncludeTrailingBackslash(GetSpecialFolder(CSIDL_APPDATA)) + ChangeFileExt(aFileName, '');
    FDataFile := IncludeTrailingBackslash(aDataPath) + aFileName;
    Result    := FileExists(FDataFile);
  end;
end;

end.

Ein Anwendung müsste dann in etwa so aussehen:

Delphi-Quellcode:

uses
  .. uDataLocation ..

var
  aDataLocation : IDataLocation;
  aDataFile : string;
begin
  aDataLocation := TDataLocation.Create;
  aDataLocation.DataFileExt('.dat')
               .ConfigFile(aDataFile);
  LoadData(aDataFile);

Ein Freigabe von aDataLocation muss nicht explizit erfolgen, weil das Interface von Delphi per RefCounting verwaltet wird.

Mit Verwendung von Spring-DI wird die Einbindung der Unit uDataLocation entfallen und auch TDataLocation.Create; wird durch einen anderen Aufruf ersetzt.

Verbesserung 1: von Furtbichler eingearbeitet, Kommentar von Uwe Raabe in uUtils.pas eingefügt
Verbesserung 2: von DeddyH eingearbeitet
Rückbau zu 1: Hinweis von Furtbichler

[Uwe Raabe]

Zitat:

Zitat von neo4a (Beitrag 1151690)

GetCmdLineSwitchValue()

Mindestens seit XE bietet Sysutils eine überladene Funktion an, die genau das bereits erledigt:

Delphi-Quellcode:

{ This version is used to return values.
  Switch values may be specified in the following ways on the command line:
    -p Value                - clstValueNextParam
    -pValue or -p:Value     - clstValueAppended

  Pass the SwitchTypes parameter to exclude either of these switch types.
  Switch may be 1 or more characters in length. }
function FindCmdLineSwitch(const Switch: string; var Value: string; IgnoreCase: Boolean = True;
  const SwitchTypes: TCmdLineSwitchTypes = [clstValueNextParam, clstValueAppended]): Boolean; overload;

[Furtbichler]

Zitat:

Delphi-Quellcode:

function TDataLocation.ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean;
var
   aFound : Boolean;
begin
   Result := False;
   if aDefaultFileName > '' then
     FDataFile := aDefaultFileName;
   if not Result then
     Result := FindDataCmd;
   if not Result then
     Result := FindConfigCmd;
   if not Result then
     Result := FindLocalConfig;
   if not Result then
     Result := FindLocalFile;
   if not Result then
     Result := FindUserDataFile;
   aConfigFileName := FDataFile;
end;

Würde ich anders schreiben, dann ist es kompakter, aber vielleicht nicht so leicht zu verstehen. BTW: Die Variable "aFound" ist überflüssig.

Delphi-Quellcode:

function TDataLocation.ConfigFile(out aConfigFileName : string; const aDefaultFileName : string = '') : Boolean;
begin
   if aDefaultFileName > '' then
     FDataFile := aDefaultFileName;
   try
     Result := True;
     If FindDataCmd then exit;
     If FindConfigCmd then exit;
     If FindLocalConfig then exit;
     If FindLocalFile then exit;
     If FindUserDataFile then exit;
     Result := False;
  finally
     aConfigFileName := FDataFile;
  end;
end;

[DeddyH]

Delphi-Quellcode:

{$BOOLEVAL OFF}
function TDataLocation.ConfigFile(out aConfigFileName : string; 
  const aDefaultFileName : string = ''): Boolean;
begin
  if aDefaultFileName > '' then
    FDataFile := aDefaultFileName;
  Result := FindDataCmd or FindConfigCmd or FindLocalConfig or FindLocalFile 
    or FindUserDataFile;
  aConfigFileName := FDataFile;
end;

Müsste auf dasselbe herauskommen.

[neo4a]

Zitat:

Zitat von DeddyH (Beitrag 1151698)

Müsste auf dasselbe herauskommen.

Sehe ich auch so: Es macht aus 7 Zeilen 1, ohne dass die Lesbarkeit wirklich leidet.

[Luckie]

Ich würde als Standarddateiname den Anwendungsnamen ohne Endung nehmen und dem Programmierer zwingen eine Endung anzugeben.

[neo4a]

Zitat:

Zitat von Luckie (Beitrag 1151701)

Ich würde als Standarddateiname den Anwendungsnamen ohne Endung nehmen und dem Programmierer zwingen eine Endung anzugeben.

Kannst Du dafür die Source-Änderung ausführen, damit ich darauf im 1. Post beim Edit verlinken kann? Irgendwie sollte ja der Prozess, das Vorher-Nachher dabei herauskommen.

[Luckie]

Puh, da müsste ich mich jetzt in den Code reinarbeiten. Aber du brauchst ParamStr(0), ChangeFileExt und ExtractFilename.

[neo4a]

Zitat:

Zitat von Luckie (Beitrag 1151704)

Puh, da müsste ich mich jetzt in den Code reinarbeiten. Aber du brauchst ParamStr(0), ChangeFileExt und ExtractFilename.

Ja, das sind aktuell 187 Zeilen ;) Wie genau stellst Du Dir das "Zwingen" über das Interface vor?

Üblicherweise würde man das wohl über den Konstruktor machen. Den wollte ich aber eigentlich so simpel belassen, damit ich dann bei der Umstellung für Spring-DI nicht gleich mit DelegateTo()-Aufrufen hantieren muss.

[Luckie]

Na ja, damit die Konfigurationsdatei oder was auch immer nicht ohne Endung im Dateisystem steht.