Cleancode, Dependency Injection oder wie stelle ich mich richtig an

[Ralle1]

Danke erstmal für das zahlreiche Feedback
Daraus fasse ich folgende Lösung für mich als am sinnvollsten zusammen:

Delphi-Quellcode:

type
   TApplicationUser = class
   private
     { Private-Deklarationen }
   public
     { Public-Deklarationen }
     class var UserId: Integer;
   end;

...

type
   TControlSettings = class
   private
     { Private-Deklarationen }
     User : TApplicationUser;
   public
     { Public-Deklarationen }
     procedure SaveSettingstoDatabase;
   end;

...

procedure TFormMain.FormCreate(Sender: TObject);
begin
  fUserID := GetGlobalUserID; //globale User ID wird von wo auch immer gesetzt bei Programmstart
  TApplicationUser.UserId := fUserId;
end;

Allerdings habe ich dabei den Ansatz von Uwe noch nicht beachtet, weil ich ihn noch nicht genau verstanden habe.
Um die Dir fehlende Info nachzureichen:
In der Methode "SaveSettingsToDatabase" sollen die Einstellungen dann in die Datenbank geschoben werden, also Query-Objekt erzeugen und ein Update/Insert auf eine Tabelle abfeuern.
Die UserId bildet dabei den Foreignkey. Über eine weitere Methode Methode "LoadSettingsFromDatabase" wird dieser Datensatz später wieder ausgelesen

[Sir Rufo]

Also du hast da irgendwelche Komponenten und da sollen Einstellungen gelesen und geschrieben werden.

Du brauchst also eine TReader und TWriter Klasse, die abstrakt Eigenschaften lesen und schreiben können. Mehr muss die Komponente doch gar nicht wissen, wohin und mit wem kann der doch egal sein.

Delphi-Quellcode:

TMyComponent = class( TComponent )
public
  procedure ReadSettings( AReader : TReader );
  procedure WriteSettings( AWriter : TWriter );
end;

Dann benötigst du eine abstrakte TSettingsStore Klasse, die eine TReader und TWriter -Instanz zur Verfügung stellt.

Jetzt werden wir konkreter:

Du leitest dir von TSettingsStore eine Klasse ab, die mit einer Datenbank kommunizieren kann und auch eine UserID aufnehmen kann.

[Ralle1]

dieser Ansatz setzt allerdings voraus, dass ReadSettings und WriteSettings von außen aufgerufen wird, oder?

Delphi-Quellcode:

procedure TFormMain.FormCreate(Sender: TObject);
begin
  ...
  MyPageControl.ReadSettings(SettingsStore.Reader);
  ...
end;

procedure TFormMain.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  ...
  MyPageControl.WriteSettings(SettingsStore.Writer);
  ...
end;

Dies ist nämlich nicht der Fall. Die Komponente kümmert sich eigenständig um das Auslösen dieser Ereignisse.

[Sir Rufo]

Und wo ist jetzt das Problem daraus Events zu machen und entsprechend zu verdrahten?

Irgendwo musst du ansetzen und etwas übergeben, ansonsten legst du dich starr fest.
Wo ist egal, und wenn die Komponenten einfach ein "Singleton" ansprechen und sich darüber die Instanz zum Speichern/Laden holen.

Diesem Singleton übergibst du dann einfach beim Start der Anwendung die konkrete Instanz und gut.

Also mal ganz simpel:

Delphi-Quellcode:

unit MyUserSettings;

interface

type
  IUserSettingsStorage = interface
    ['{F3E5657B-39EF-4FA1-A601-8AFCEE50D6D1}']
    procedure WriteString( const AName, AValue : string );
    function ReadString( const AName, ADefault : string ) : string;
  end;

  TUserSettings = class
  private
    class var _Storage : IUserSettingsStorage;
  private
    class function GetStorage : IUserSettingsStorage; static;
    class procedure SetStorage( const Value : IUserSettingsStorage ); static;
  public
    class property Storage : IUserSettingsStorage read GetStorage write SetStorage;
  end;

implementation

{ TUserSettings }

class function TUserSettings.GetStorage : IUserSettingsStorage;
begin
  // Wenn _Storage NIL, dann könnte man auch eine DUMMY/NULL-Instanz zurückgeben, die einfach nichts macht
  // dann spart man sich die Überprüfung, ob es eine Instanz gibt
  Result := _Storage;
end;

class procedure TUserSettings.SetStorage( const Value : IUserSettingsStorage );
begin
  _Storage := Value;
end;

end.

und in der Komponente dann

Delphi-Quellcode:

interface

type
  TMyComponent = class(...)
  protected
    procedure DoLoadUserSettings;
    procedure DoSaveUserSettings;
  end;

implementation

uses
  MyUserSettings;

procedure TMyComponent.DoSaveUserSettings;
begin
  TUserSettings.Storage.WriteString( 'Name', Name );
end;

procedure TMyComponent.DoLoadUserSettings;
begin
  Name := TUserSettings.Storage.ReadString( 'Name', Name );
end;

Es gibt da natürlich noch eine ganze Bandbreite an Spielmöglichkeiten.

Die Storage gibt mir eine spezielle Instanz für die Komponente zurück TUserSettings.Storage('TForm1.PageControl1').WriteString('Name',Name); etc., etc.

[Ralle1]

Das hört sich sehr gut an. So werde ich es mal angehen. Vielen dank an alle für das Feedback!!!

[Stevie]

Zitat von Ralle1:

Die Komponente arbeitet mit einer Klasse in der die Speicherung der Einstellungen ausgelagert ist. Zur eindeutigen Speicherung (an einem zentralen Ort, Datenbank) benötige ich u.a. eine User-ID aus der Anwendung.

Imo ist die Annahme, dass die Einstellungen eine UserID brauchen schonmal eine aufgezwungene Kopplung, die nicht notwendig ist. Die Komponente interessieren die Einstellungen, ob die einem User zugehörig oder in ne Ini Datei auf C geschrieben werden, ist unerheblich. Dementsprechend sollte die Komponente nur die Settings Schnittstelle/Instanz interessieren, aus der es befüttert wird.

@Sir Rufo:
Naja, ist immer noch gekoppelt, nur nett als Klasse mit class var verpackt, dass es ein global State ist, ändert sich nicht. Imo kann man doch locker ein Settings Objekt/Komponente basteln, die man an das PageControl, was die Settings gerne hätte heften kann, genau wie man normalerweise Komponenten aneinander steckt. Dann hat man sogar den Vorteil, dass man übermorgen, wenn mal jemand die verrückte Idee hat 2 verschiedene PageControls unterschiedlich zu konfigurieren, nicht dumm da steht.

P.S.: Weil ich das Video zu dem Thema so gut finde und so gerne verlinke: https://www.youtube.com/watch?v=-FRm3VPhseI&t=1s

[Dejan Vu]

Zitat von Stevie:

Imo ist die Annahme, dass die Einstellungen eine UserID brauchen schonmal eine aufgezwungene Kopplung, die nicht notwendig ist.

Die Einstellungen benötigen das schon, sofern wir beide von den gleichen 'Einstellungen' reden. Ich definiere 'Einstellungen' als 'Settings' und so wie der Terminus hier verwendet wird, scheint es sich um den Kontext zu handeln, der die Properties der Komponente liest und schreibt und *nicht* um die Properties der Komponente (so wie Du das vielleicht definierst).

Die Komponente an sich braucht und darf die UserId nicht kennen, denn sonst könnte sie ohne nicht leben. Ich würde sogar soweit gehen, das 'LoadFrom(aReader : TReader)' und 'SaveTo(aWriter : TWriter)' aus der Komponente zu entfernen. Die Komponente muss nicht lesen und schreiben können. Wir Menschen können ja auch Bäume fällen, Brote essen und an Blümchen Spaß haben, ohne lesen und schreiben zu können (nur der Wille zum neuen Leben als Holzfäller ist hier maßgebend).

Insofern würde ich die Komponente Komponente sein lassen, schlank und so, wie GottProgrammierer sie schuf.

Und wenn nun irgendwer meint, die Eigenschaften irgendwohin speichern zu müssen, soll er das doch tun. Das schreit dann geradezu nach einer TReader und TWriterFactory, sodaß diese Factory für jedes Control den passenden Reader/Writer aus dem Hut zaubert.

Und da wir die Factory wieder über eine Factory erzeugen lassen können, können wir hier dann unsere speziellen DB-ReaderWriter über eine entsprechende Factory erzeugen lassen. D.h. ich habe dann die Freiheit, heute mal in die Registry und morgen von mir aus in die DB (oder eine Textdatei oder oder oder) zu speichern, wobei nur bei der DB-Variante wirklich ausnahmsweise eine UserID nötig wäre. Bei anderen benötigen wir vielleicht irgendeinen Pfad, oder einen Registry-Schlüssel etc. Das ist dann komplett entkoppelt und die spezielle DB-ReaderWriter-Factory kennt dann die UserID und gibt sie bei der Produktion einer Klasse dieser eben mit. Und somit wäre auch das don't-use-globals gelöst, denn global ist die User-ID dann ja nun nicht mehr.

[Ralle1]

Die letzten Antworten haben mich nun zugegebener Weise mehr verwirrt als geholfen. Das liegt aber wohl zuletzt daran, dass ich noch nicht ganz folgen kann.

Ich habe mir mal die Mühe gemacht, das Problem in einem Demoprojekt nachzubauen. Siehe Anhang.
Dort gibt es:

1. Anwendung: das RememberProject besteht aus den Formularen Main und Detail. Beim Start des FormMain wird quasi als Login eine UserId abgefragt und global gespeichert. Dies simuliert an der Stelle den Bestandscode. Sicher nicht ganz optimal, aber halt der Ist-Zustand.
Das FormDetail bietet nun als Komponente ein Edit (TRememberEdit) das sich je UserId den eingegebenen Text merkt (ganz tolles Feature ).

2. Komponente: im RememberPackage ist die o.g. TRememberEdit Komponente enthalten. Über die Klasse TControlSettings wird das speichern/lesen realisiert. Diese wiederum arbeitet mit TApplicationUser um von dort die User-Id zu holen.

Soweit mein Lösungsansatz. Jetzt würde mich interessieren, ob mir jemand die „saubere“ Lösung als Alternative präsentieren kann.

@Stevie: besonders Deine Lösung würde mich interessieren! Deine Denkansätze beim Delphi-Code-Camp haben mir grundsätzlich gut gefallen, nur hapert es mir hier im konkreten Beispiel bei der Umsetzung...

Vielen Dank!

[TiGü]

Wenn du auf die globale Variable der User-ID im Hauptformular (Main.pas) verzichtest und im Unterformular (Detail.pas) nicht das Hauptformular inkludierst, sondern gleich ApplicationUser, wird das eine runde Sache.
Außerdem kann man RememberEdit noch ein bisschen straffen, indem man die Settings-Klasse als Member-Variable anlegt.
So kommt es wegen jeder kleinen Änderung nicht ständig zum Erschaffen und Zerstören der TControlSettings-Klasse.
Sicher gibt es noch ein paar andere und ausgefallende Ansätze, aber im Wesentlichen hast du damit erreicht was du wolltest: Die Komponente kennt nicht mehr die UserID.
Ggf. könnte man die UserID per Property o.ä. an das Unterformular (Detail.pas) übergeben, dann muss man hier auch nichts mehr inkludieren. Also weder Main noch ApplicationUser.

Delphi-Quellcode:

unit Main;

interface

uses
  System.SysUtils,
  System.Classes,
  Vcl.Controls,
  Vcl.Forms,
  Vcl.Dialogs,
  Vcl.StdCtrls;

type
  TFormMain = class(TForm)
    Button1 : TButton;
    procedure Button1Click(Sender : TObject);
    procedure FormShow(Sender : TObject);
  private
    function GetUserId : integer;
    function CheckUserId : boolean;
  public
  end;

var
  FormMain : TFormMain;

implementation

{$R *.dfm}


uses
  Detail,
  ApplicationUser;

procedure TFormMain.Button1Click(Sender : TObject);
var
  FormDetail : TFormDetail;
begin
  FormDetail := TFormDetail.Create(Self);
  try
    FormDetail.ShowModal;
  finally
    FreeAndNil(FormDetail);
  end;
end;

function TFormMain.CheckUserId : boolean;
begin
  Result := TApplicationUser.UserId >= 0;
  if not Result then
  begin
    MessageDlg('UserID ungültig', mtError, [mbOk], 0);
  end;
end;

procedure TFormMain.FormShow(Sender : TObject);
begin
  TApplicationUser.UserId := GetUserId;
  if not CheckUserId then
    Application.Terminate;

  Self.Caption := 'Login mit UserID=' + IntToStr(TApplicationUser.UserId);
end;

function TFormMain.GetUserId : integer;
var
  UserString : string;
begin
  Result := 0;
  UserString := InputBox('User-ID', 'Bitte die User-ID eingeben:', '0');
  Result := StrToIntDef(UserString, 0);
end;

end.

Delphi-Quellcode:

unit Detail;

interface

uses
  System.SysUtils,
  System.Classes,
  Vcl.Controls,
  Vcl.Forms,
  Vcl.StdCtrls,
  RememberEdit;

type
  TFormDetail = class(TForm)
    Label1 : TLabel;
    RememberEdit1 : TRememberEdit;
    lblHallo : TLabel;
    procedure FormCreate(Sender : TObject);
  private
  public
  end;

implementation

{$R *.dfm}

uses
  ApplicationUser;

procedure TFormDetail.FormCreate(Sender : TObject);
begin
  lblHallo.Caption := lblHallo.Caption + ' ' + IntToStr(ApplicationUser.TApplicationUser.UserId);
end;

end.

Delphi-Quellcode:

unit RememberEdit;

interface


uses
  SysUtils,
  WinTypes,
  WinProcs,
  Messages,
  Classes,
  Vcl.StdCtrls,
  ControlSettings;

type
  TRememberEdit = class(TEdit)
  private
    FControlSettings : TControlSettings;
    procedure SetTextToSettings(const Value : string);
    function GetTextFromSettings : string;
  public
    constructor Create(AOwner : TComponent); override;
    destructor Destroy; override;
  end;

procedure Register;

implementation

procedure Register;
begin
  RegisterComponents('RememberTest', [TRememberEdit]);
end;

constructor TRememberEdit.Create(AOwner : TComponent);
begin
  inherited;
  FControlSettings := TControlSettings.Create;
  Self.Text := GetTextFromSettings;  
end;

destructor TRememberEdit.Destroy;
begin
  SetTextToSettings(Self.Text);
  FControlSettings.Free;
  inherited;
end;

function TRememberEdit.GetTextFromSettings : string;
begin
  Result := '';
  if (csDesigning in ComponentState) then
    exit;
  Result := FControlSettings.Read;
end;

procedure TRememberEdit.SetTextToSettings(const Value : string);
begin
  if (csDesigning in ComponentState) then
    exit;
  FControlSettings.Write(Value);
end;

end.

[Dejan Vu]

Delphi-Quellcode:

... finally
     FreeAndNil(FormDetail);
   end;

Ich würde da 'Release' verwenden. Soweit ich mich erinnere, ist das der bevorzugte Weg, ein Windows-Formular freizugeben.

Und nochwas: Die Logik von CheckUserId gehört nicht ins Login/Hauptformular, imho.