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

[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.

[Stevie]

Zitat von Ralle1:

@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...

Danke - ich freu mich, wenn ich zumindest einen kleinen Denkanstoß geben konnte

Ist noch nicht die 100%ige Lösung, da mir die Abhängigkeit vom Detailform auf das Mainform noch nicht gefällt (siehe Kommentar im Source).
Außerdem wird durch das Fehlen einer Abstrakten Settings Klasse noch die konkrete Implementierung vom TRememberEdit benötigt - da sollte man dann noch eine abstrakte Klasse implementieren (ich weiß grad nicht, ob man auch Interfaces im OI verdrahten kann).

Den noch etwas unsauberen Code im Mainform hab ich mal so belassen, denn darum ging es ja nicht hauptsächlich.

[TiGü]

Zitat von Stevie:

Ist noch nicht die 100%ige Lösung, da mir die Abhängigkeit vom Detailform auf das Mainform noch nicht gefällt (siehe Kommentar im Source).
Außerdem wird durch das Fehlen einer Abstrakten Settings Klasse noch die konkrete Implementierung vom TRememberEdit benötigt - da sollte man dann noch eine abstrakte Klasse implementieren (ich weiß grad nicht, ob man auch Interfaces im OI verdrahten kann).

Den noch etwas unsauberen Code im Mainform hab ich mal so belassen, denn darum ging es ja nicht hauptsächlich.

Warum kennt bei dir die Mainform und die Detailform die ControlSettings für das RememberEdit?
Geht es nur darum, dass fancy im OI zu verdrahten?
Da fand ich meine Lösung sauberer.

[Dejan Vu]

Zitat von Stevie:

Ist noch nicht die 100%ige Lösung, da mir die Abhängigkeit vom Detailform auf das Mainform noch nicht gefällt (siehe Kommentar im Source).

a) Entkoppel doch einfach die Settings komplett von der Komponente
a.1) Lass das TFormDetail.FormCreate ein Settings.Load(RememberEdit) ausführen
a.2) Schreib eine TSettings-Komponente, klatsch die auf das TFormDetail und füge die 'RememberEdit'-Komponente der TSettings-Komponente zu (so macht das DevExpress mit seinem TPropertyStore oder wie das Teil heißt).
b) So, wie Du das schon angedacht hast: Settings => Datamodule

Ich find a besser, denn die Lösung geht mit allen Komponenten. Bei eurer bisherigen Lösung muss man zwangsweise für jede Komponente, die ihre Settings persistiert, eine Ableitung machen in meinen Augen.