Wäre die Situation aus Clean Code Sicht eigentlich okay, wenn ich derartige Variablen als strict private class var deklariere oder ist das auch schon grenzwertig und nur verschlimmbesserte globale Variablen?

Ich verstehe nicht, warum du dir deine Instanz überhaupt in der Dll merken willst. Wozu? Soll es nur eine (eine bestimmte Anzahl) gleichzeitig geben können? Dann mach doch einen Zähler.
Willst du ein Singleton? Dann merke es dir doch als Klassentyp (TDataManager), denn du weißt ja in der Dll die konkrete Klasse.

Also für den "DataManager" in diesem Beispiel war das natürlich Quatsch.
Hier ist es wirlich besser die einzige lebende Instanz im Hauptprogramm zu halten.

Aber wie ist es bei der Verwaltung von Hardware-Ressourcen?
In meiner richtigen Problemstellung steuere und erhalte ich Messwerte von einer externen Hardware per PCI-Karte (http://www.adlinktech.com/PD/web/PD_detail.php?pid=20).

Hier habe ich mir eine Klasse als zentrale Verwaltungseinheit geschrieben (nennen wir sie CardManager), die erstmal nach dem Erzeugen (AfterConstruction) guckt, wieviele Karten es so gibt und welche Funktion sie erhalten sollen (abspeichern in einen Dictionary).
Das Suchen und Zuweisen von Funktionen soll natürlich nur einmal geschehen.

Eine abstrakte Factory-Klasse erhält eine class var auf diese CardManager-Klasse und kann von außen abgefragt werden, wenn auf die Ressourcen zugegriffen werden soll.

Hm, also ich tue mich einfach nur ein bisschen schwer eine vernünftige Struktur zu entwerfen, die wasserdicht und relativ zukunftsicher ist.
Du musst anscheinend in deiner beruflichen Problemstellung auch Werte von externen Messgeräten verarbeiten.
Wie handhabt ihr das so? Gibt es da irgendwelche empfehlenswerte Literatur/Websiten?

Ganz verstanden habe ich es noch nicht. Der CardManager ist ein Singleton, richtig? Der kümmert sich einmalig um alle angeschlossenen Systeme ("soll natürlich nur einmal geschehen").

Und diese Instanz möchtest du als COM-Interface rausgeben. Sieht doch alles super aus finde ich.

Etwas off-Topic:
Eins habe ich anfangs auch immer gemacht, mittlerweile bereue ich es heftigst: Im Konstruktor eines Objekts anfangen, irgendwelche Dinge zu tun. Hier sogar Kommunikation starten, auswerten, was auch immer. Das ist einer der heftigsten Fehler den ich durchgehend getan habe.

Eine Zusammenfassung in ein paar Zeilen warum nicht liefert der Google C++ Styleguide: Link.
Auf Delphi trifft das natürlich genauso zu.

Wochenend-(Urlaub?)Lesestoff dazu: Link.

Ob du das nun im constructor oder im AfterConstruction machst ist kein Unterschied. Warum überhaupt AfterConstruction ?

Das mit dem Konstruktor muss ich mal in Ruhe durchdenken, dass mache ich nämlich häufig und könnte wirklich zu einem Problem werden.

Ich habe mein Anlegen in einen hoffentlich verständlichen Beispiel runtergetippt.
Leider ist es etwas länger geworden.

Im folgenden Konsolenprogramm geht es um einen Waschsalon.
Man kann von außen angeben, wieviele Waschmaschinen der Salon hat.

Die Hardware-Implementation einer einzelnen Waschmaschine soll für diese aber egal sein. Sie muss nicht wissen, wie und mit was sie kommuniziert.
Daher habe ich ein Interface IHardwareAnsteuerung als Field deklariert.
Im Beispiel ist die Hardwareansteuerung per PCI-Karte implementiert (THardwareAnsteuerungPerPCI_Karte ), aber es sollen zukünftig noch andere Verfahren möglich sein (USB, Ethernet, Bluetooth, WLAN).
Am Ende geht es um Daten die zu der Waschmaschine hingehen ('Jetzt mal waschen') und Daten die abgefragt werden ('Wie lange ist denn noch?', 'Wie warm ist das Wasser?, 'Wo ist die rote Socke in der Trommel?').

Hier ist auch der Punkt mit der class var in der THardwareAnsteuerungFactory eingebaut.
Ob ich jetzt wirklich ein Singleton brauche ist fraglich...hm, mal am Wochenende drüber nachdenken!

Ist grundsätzlich meine Struktur tragbar?

zusammenfalten · markieren

Delphi-Quellcode:

program Waschsalon;

{$APPTYPE CONSOLE}

{$R *.res}


uses
  System.SysUtils,
  System.Classes,
  System.Math,
  System.Generics.Collections;

type
  TPCICardID = record
  private
    FBits : LongWord;
  public
    procedure SetBits(const Bits : LongWord);
    function GetBits : LongWord;
  end;



  IHardwareAnsteuerung = interface
    ['{C279C43A-E9E5-4E2D-96B5-A9CAD9E8A118}']
    procedure MachWaschen;
    procedure MachSchleudern;
    procedure MachRödeln;
  end;

  THardwareAnsteuerungPerPCI_Karte = class(TInterfacedObject, IHardwareAnsteuerung)
  private
    FPCICardID : TPCICardID;
  public
    constructor Create(const CardID : TPCICardID);
    procedure MachWaschen;
    procedure MachSchleudern;
    procedure MachRödeln;
  end;



  ICardManager = interface
    ['{6839D9A7-295A-4356-8B69-FD6BB1522280}']
    function GibSchonHerDieHardware : IHardwareAnsteuerung;
  end;

  TCardManager = class(TInterfacedObject, ICardManager)
  private
    FCard : TPCICardID;
  public
    procedure AfterConstruction; override;
    procedure InitializePCICard;
    function GibSchonHerDieHardware : IHardwareAnsteuerung;
  end;

  THardwareAnsteuerungFactory = class abstract
  strict private
    class var _CardManager : ICardManager;
  public
    class function GetHardwareAnsteuerung(out HardwareAnsteuerung : IHardwareAnsteuerung) : Boolean;
  end;



  IWaschmaschine = interface
    ['{79EA598A-4A88-4D0A-BC99-DDF28ECC95F3}']
    procedure Waschen;
    procedure Schleudern;
    procedure NurSoRumRödeln;
  end;

  TWaschmaschine = class(TInterfacedObject, IWaschmaschine)
  strict private
    class var FWaschmaschinenAnzahl : Integer;
  private
    FWaschmaschinenNummer : Integer;
    FHardwareansteuerung : IHardwareAnsteuerung;
  public
    constructor Create(const HardwareAnsteuerung : IHardwareAnsteuerung);
    procedure Waschen;
    procedure Schleudern;
    procedure NurSoRumRödeln;
    procedure AfterConstruction; override;
  end;



  IWaschsalon = interface
    ['{240F248A-E379-44B3-ADBA-B08B3DE26567}']
    function GibtWaschmaschineMitDerNummer(WaschmaschinenNummer : Integer) : IWaschmaschine;
  end;

  TWaschsalon = class(TInterfacedObject, IWaschsalon)
  private
    FWaschmaschinenAnzahl : Integer;
    FWaschmaschinenListe : TList<IWaschmaschine>;
    procedure FülleWaschmaschinenListe;
  public
    constructor Create(const WaschmaschinenAnzahl : Integer);
    function GibtWaschmaschineMitDerNummer(WaschmaschinenNummer : Integer) : IWaschmaschine;
    procedure AfterConstruction; override;
    procedure BeforeDestruction; override;
  end;

  { TWaschmaschine }

procedure TWaschmaschine.AfterConstruction;
begin
  inherited;
  FWaschmaschinenNummer := FWaschmaschinenAnzahl;
  Inc(FWaschmaschinenAnzahl);
end;

constructor TWaschmaschine.Create(const HardwareAnsteuerung : IHardwareAnsteuerung);
begin
  FHardwareansteuerung := HardwareAnsteuerung;
end;

procedure TWaschmaschine.NurSoRumRödeln;
begin
  Writeln(FWaschmaschinenNummer.ToString + ' Rödeln!');
  FHardwareansteuerung.MachRödeln;
end;

procedure TWaschmaschine.Schleudern;
begin
  Writeln(FWaschmaschinenNummer.ToString + ' Schleudern!');
  FHardwareansteuerung.MachSchleudern;
end;

procedure TWaschmaschine.Waschen;
begin
  Writeln(FWaschmaschinenNummer.ToString + ' Waschen!');
  FHardwareansteuerung.MachWaschen;
end;


{ TWaschsalon }

procedure TWaschsalon.AfterConstruction;
begin
  inherited;
  FWaschmaschinenListe := TList<IWaschmaschine>.Create;

  FülleWaschmaschinenListe;
end;

procedure TWaschsalon.BeforeDestruction;
begin
  inherited;
  FWaschmaschinenListe.Free;
end;

constructor TWaschsalon.Create(const WaschmaschinenAnzahl : Integer);
begin
  FWaschmaschinenAnzahl := WaschmaschinenAnzahl;
end;

procedure TWaschsalon.FülleWaschmaschinenListe;
var
  LWaschmaschine : IWaschmaschine;
  I : Integer;
  LHardwareAnsteuerung : IHardwareAnsteuerung;
begin
  if THardwareAnsteuerungFactory.GetHardwareAnsteuerung(LHardwareAnsteuerung) then
  begin
    for I := 1 to FWaschmaschinenAnzahl do
    begin
      LWaschmaschine := TWaschmaschine.Create(LHardwareAnsteuerung);
      FWaschmaschinenListe.Add(LWaschmaschine);
    end;
  end;
end;

function TWaschsalon.GibtWaschmaschineMitDerNummer(WaschmaschinenNummer : Integer) : IWaschmaschine;
begin
  if InRange(WaschmaschinenNummer, 0, FWaschmaschinenListe.Count) then
  begin
    Result := FWaschmaschinenListe.Items[WaschmaschinenNummer];
  end;
end;

{ THardwareAnsteuerung }

constructor THardwareAnsteuerungPerPCI_Karte.Create(const CardID : TPCICardID);
begin
  FPCICardID := CardID;
end;

procedure THardwareAnsteuerungPerPCI_Karte.MachRödeln;
begin
  FPCICardID.SetBits($0123);
end;

procedure THardwareAnsteuerungPerPCI_Karte.MachSchleudern;
begin
  FPCICardID.SetBits($0456);
end;

procedure THardwareAnsteuerungPerPCI_Karte.MachWaschen;
begin
  FPCICardID.SetBits($0789);
end;

{ THardwareAnsteuerungFactory }

class function THardwareAnsteuerungFactory.GetHardwareAnsteuerung(out HardwareAnsteuerung : IHardwareAnsteuerung) : Boolean;
begin
  if not Assigned(_CardManager) then
  begin
    _CardManager := TCardManager.Create;
  end;

  HardwareAnsteuerung := _CardManager.GibSchonHerDieHardware;
  Result := Assigned(HardwareAnsteuerung);
end;

{ TCardManager }

procedure TCardManager.AfterConstruction;
begin
  InitializePCICard;
end;

function TCardManager.GibSchonHerDieHardware : IHardwareAnsteuerung;
begin
  Result := THardwareAnsteuerungPerPCI_Karte.Create(FCard);
end;

procedure TCardManager.InitializePCICard;
begin
  FCard.SetBits($01010101);
end;

{ TPCICardID }

function TPCICardID.GetBits : LongWord;
begin
  Result := FBits;
end;

procedure TPCICardID.SetBits(const Bits : LongWord);
begin
  FBits := FBits or Bits;
end;

procedure Main;
var
  LWaschsalon : IWaschsalon;
  LWaschmaschine : IWaschmaschine;
  WaschmaschinenAnzahl, I : Integer;
begin
  Writeln('Wieviele Waschmaschinen sind im Waschsalon?');
  Readln(WaschmaschinenAnzahl);

  LWaschsalon := TWaschsalon.Create(WaschmaschinenAnzahl);
  for I := 0 to WaschmaschinenAnzahl - 1 do
  begin
    LWaschmaschine := LWaschsalon.GibtWaschmaschineMitDerNummer(I);

    LWaschmaschine.Waschen;
    LWaschmaschine.NurSoRumRödeln;
    LWaschmaschine.Schleudern;
  end;
end;

begin
  try
    Main;
    Readln;
  except
    on E : Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;

end.

Es gibt eine zentrale Instanze "Waschsalon" die für die Anwendung "Waschmaschinen" bereitstellt.

Der "Waschsalon" bekommt seine "Waschmaschinen" von der "HardwareAnsteuerungFactory", davon kann es aber mehrere geben.
Eine für PCI, USB, WLAN oer was uns die Zukunft noch so alles bringt.
Eventuell werden diese auch erst zur Laufzeit der Anwendung per DLL eingebunden(z.B. eine DLL je Hardwaretyp).

Der "Waschsalon" braucht nicht zu wissen, woher die "Waschmaschinen" kommen.
Deshalb ist es sinnvoll, das jede "HardwareAnsteuerungFactory" ihre "Waschmaschinen" erzeugt und beim "Waschsalon" registriert.

zusammenfalten · markieren

Delphi-Quellcode:

IWaschsalon = interface
  ['{240F248A-E379-44B3-ADBA-B08B3DE26567}']
  function GibtWaschmaschineMitDerNummer(WaschmaschinenNummer : Integer) : IWaschmaschine;
  
  function Count: Integer; // Anzahl der registrierten Maschinen
  function GetItem(AIndex: Integer): IWaschmaschine;
  procedure Register(AWaschmaschine: IWaschmaschine); // Maschine hinzufügen
end;


THardwareAnsteuerungFactory.RegisterWaschmaschinen(AWaschsalon: IWaschsalon);
var
  Waschmaschine: TWaschmaschine;
begin
  for i := 0 to FItems.Count - 1 do
  begin
    Waschmaschine := TWaschmaschine.Create(FItems[i]);
    AWaschsalon.Register(Waschmaschine);
  end;
end;

Hallo Blup,

vielen Dank für die Anregung.
Da es sich aber irgendwie falsch anfühlt, dass die HardwareAnsteuerungsFactory sowohl darüber entscheidet, welche Art von Ansteuerung verwendet wird, als auch die Waschmaschinen kennt und erzeugt, habe ich die Register-Methode in eine eigene WaschmaschinenFactory ausgelagert.
Es erscheint mir noch flexibler zu sein.

Noch ein bisschen unzufrieden bin ich mit dem Punkt, den der Günther erwähnt hat.
Ich veranlasse das Erzeugen im AfterConstruction des Waschsalons und bin damit jeder Exception schutzlos ausgeliefert.
Es wäre wohl besser dem Befehl des Erzeugens eine Ebene höher explizit auszuführen.
Hier im Beispiel also in der procedure Main nach dem Erzeugen des Waschsalons.

Was denkt ihr?
Im Anhang der aktuelle Stand.

zusammenfalten · markieren

Delphi-Quellcode:

  IWaschsalon = interface
    ['{240F248A-E379-44B3-ADBA-B08B3DE26567}']
    function GibtWaschmaschineMitDerNummer(WaschmaschinenNummer : Integer) : IWaschmaschine;
    function GetCount: Integer;
    function GetMaxWaschmaschinen : Integer;
    procedure RegisterWaschmaschine(const AWaschmaschine: IWaschmaschine);

    property MaxWaschmaschinen : Integer read GetMaxWaschmaschinen;
    property Count : Integer read GetCount;
  end;

...

  TWaschmaschinenFactory = class abstract
    class procedure RegisterWaschmaschinen(const AWaschsalon: IWaschsalon);
  end;

...

class procedure TWaschmaschinenFactory.RegisterWaschmaschinen(const AWaschsalon: IWaschsalon);
var
  Waschmaschine: IWaschmaschine;
  HardwareAnsteuerung : IHardwareAnsteuerung;
  I : Integer;
begin
  THardwareAnsteuerungFactory.GetHardwareAnsteuerung(HardwareAnsteuerung);

  for I := 0 to AWaschsalon.MaxWaschmaschinen - 1 do
  begin
    Waschmaschine := TWaschmaschine.Create(HardwareAnsteuerung);
    AWaschsalon.RegisterWaschmaschine(Waschmaschine);
  end;
end;

...

procedure TWaschsalon.AfterConstruction;
begin
  inherited;
  FWaschmaschinenListe := TList<IWaschmaschine>.Create;

  FülleWaschmaschinenListe;
end;

...

procedure TWaschsalon.FülleWaschmaschinenListe;
begin
  TWaschmaschinenFactory.RegisterWaschmaschinen(Self);
end;