Getter wird übergangen

[Medium]

Ahoi!

Ich habe hier folgende Struktur (eine Objekt-Relationale Abbildung zu einer DB, Delphi 2010):

Delphi-Quellcode:

  TDB = class
  private
    type
      TDBUser = class
      private
        class var FID: Integer;
        class function GetName: string; static;
        class procedure SetName(const Value: string); static;
        // für jedes DB feld eben...
      public
        class property ID: Integer read FID;
        class property Name: string read GetName write SetName;
      end;
    class var FQuery: TUniQuery;
    class var FUser: TDBUser;
    class function GetDBUserByLogin(aLogin: string): TDBUser; static;
  public
    class property UsersByLogin[aLogin: string]: TDBUser read GetDBUserByLogin;
  end;

implementation

class function TDB.GetDBUserByLogin(aLogin: string): TDBUser;
begin
  Query.SQL.Text := 'SELECT id FROM users WHERE login=:lg';
  Query.ParamByName('lg').AsString := aLogin;
  Query.Open;
  if Query.RecordCount=0 then
  begin
    Query.Close;
    raise EUserDoesNotExistException.Create();
  end;
  FUser.FID := Query.FieldByName('id').AsInteger;
  result := FUser;
  Query.Close;
end;

Um die ID eines Users anhand seines Logins zu erhalten, tue ich in einer anderen Unit dies:

Delphi-Quellcode:

constructor TUser.Create(aLogin, aPassword: string);
var
  id: Integer;
begin
  if FInstanceCounter > 0 then
    raise Exception.Create('Es ist bereits ein User angemeldet. Melden Sie diesen ab, bevor Sie sich einloggen.');

  try
    id := TDB.UsersByLogin[aLogin].ID; // !!! Hier wichtig !!! :)
  except
    raise Exception.Create('Benutzer '''+aLogin+''' existiert nicht.');
  end;

  if TDB.Users[id].Password <> aPassword then
    raise Exception.Create('Ungültiges Passwort für Benutzer '''+aLogin+'''.');

  FId      := id;
  FLogin   := aLogin;
  FName    := TDB.Users[id].Name;
  FSurname := TDB.Users[id].Surname;
  FRights  := TDB.Users[id].Rights;

  inc(FInstanceCounter);
end;

An der markierten Stelle passiert es dann: NICHTS!
Es wird niemals die Methode GetDBUserByLogin() durchlaufen, und "id" hat nachher immer den Wert "0" (diese ID gibt es nicht in der DB). Ich erhalte keine Hinweise/Warnungen vom Compiler, kann einen Haltepunkt auf die oben markierte Zeile setzen, ein solcher wird in der Methode GetDBUserByLogin() jedoch deaktiviert. (Das ist die einzige Überladung davon.)
Löschen der DCUs hat leider nichts gebracht, sowie auch das testweise entfernen des try-Blocks. Auch die Optimierung abzuschalten hat an diesem Verhalten nichts geändert.

Bin es nun ich, der einen Denkfehler hat und ihn nicht findet, oder wäre das es wert dem Support vorzutragen? :gruebel:

Besten Dank schon mal!
Medium

[shmia]

Wieso hast du denn ausschliesslich statische class-Methoden?
Das ist sehr unschön und damit verbaust du Dir alle Möglichkeiten für die Zukunft.
Die Klasse TDBUser sollte auf jeden Fall eine ganz normale Klasse sein; ungefähr so:

Delphi-Quellcode:

TDBUser = class
private
 FID: Integer;
 FName : string;
public
  property ID: Integer read FID;
  property Name: string read GetName write SetName;
  property LastLogon:TDateTime read FLastLogon;
end;

Die Funktion GetDBUserByLogin() sieht dann ungefähr so aus:

Delphi-Quellcode:

class function TDB.GetDBUserByLogin(aLogin: string): TDBUser;
begin
  Query.SQL.Text := 'SELECT id FROM users WHERE login=:lg';
  Query.ParamByName('lg').AsString := aLogin;
  Query.Open;
  if Query.IsEmpty {besser als RecordCount=0} then
  begin
    Query.Close;
    raise EUserDoesNotExistException.Create(aLogin {welcher user nicht existiert ist wichtig zu wissen});
  end;
  result := TDBUser.Create;  // neues Obj erzeugen !
  result.ID   := Query.FieldByName('id').AsInteger;
  result.Name := Query.FieldByName('Name').AsString;
  ....
  Query.Close;
end;

Der Aufrufer GetDBUserByLogin() muss dann das TDBUser-Objekt freigeben.
Falls Dir das nicht gefällt, kann man auch auf Interfaces ausweichen.
Damit werden die TDBUser-Objekte automatisch freigegeben (über die Referenzzählung).

[Medium]

Eben genau die Freigabefrage war der Grund für die Klassenmethoden. Es würde ja praktisch für jeden Zugriff auf einen User, wie klein auch immer, eine neue Instanz erstellt, die dann irgendwer freigeben müsste. Da die Instanz selber aber niemanden interessiert, sondern nur das, was die Properties zurück geben (das sind intern SQL Abfragen), wird auch nirgends eine Referenz auf diese behalten. Das möchte ich auch keinesfalls!

Da mir Interfaces recht unsympathisch sind, muss ich mal fragen, was ich mir so verbaue? Ableiten soll da ohnehin nie mehr jemand von, deswegen ist es ja eine private Klasse, und ich sollte sie ggf. auch noch sealed machen. Das ist auf eine ganz spezielle DB zugeschnitten, und wer auch immer darin etwas ändert hat a) auch Zugang zum Source, und bekommt b) was auffe Löffel :).

Das seltsame Verhalten erklärt das allerdings leider noch nicht :?

Edit: IsEmpty ist schöner, danke! Welcher User da nicht gefunden wurde, wird im Moment eine Aufruftiefe höher behandelt. Da weiss ich ja auch mit welchem Login ich nachgefragt habe, und on EUserDoesNotExistException wird jetzt explizit mit einem MessageDlg behandelt. Man könnte das noch als Parameter an die Exception geben, Sinn würd das auch machen. Denken :) Macht viel Sinn, habs angepasst.

[himitsu]

OK, statt dem Interface würde ein Record das selbe Ergebnis bieten.
- mehrere solcher Container gleichzeitig möglich
- Delphi kümmert sich automatisch um die Freigabe

Delphi-Quellcode:

TDBUser = record
private
  FID: Integer;
  FName: string;
  FLastLogon: TDateTime;
public
  property ID: Integer read FID;
  property Name: string read FName;
  property LastLogon: TDateTime read FLastLogon;
end;

[edit]
in Delphi 7 dann ein normaler Record ohne Property (die waren eh nur für ein ReadOnly gedacht)

[Medium]

Das könnte sicherlich auch gut klappen, und wäre ggf. eine Überlegung wert. Ich hatte beim Basteln .NET im Hinterkopf, wo sowas dank GC ja usus ist.

Allerdings hilft mir das bei meinem eigentlichen Problem kein Stück weiter, da der Getter von TDB, nicht von TDB.TDBUser betroffen ist, also eine Ebene höher. Welchen Typ TDBUser dann hat, ist wohl relativ unerheblich hier, auch wenn ich die Ideen gerne annehme! Das hat nur leider nix mit dem Problem an sich zu tun :?

[himitsu]

Erstmal wäre es gut, daß du erwähnst, wenn du eine andere Delphiversion, als die im Profil erwähnte, verwendest.
(inline-Typen gingen in D7 doch noch garnicht)
ups, übersehn :oops:

Dann kann es sein, daß der Compiler, Aufgrund deiner komischen Sichtbarkeitsreglungen, durchdreht.
Dein TDBUser ist als Private deklariert, aber du willst es in Public verwenden, als Result für eine Public-Methode/Property.

[DeddyH]

Delphi 2010 steht aber ganz klar im Ausgangspost :zwinker:

[Medium]

Zitat:

Zitat von himitsu (Beitrag 1029743)

Dann kann es sein, daß der Compiler, Aufgrund deiner komischen Sichtbarkeitsreglungen, durchdreht.
Dein TDBUser ist als Private deklariert, aber du willst es in Public verwenden, als Result für eine Public-Methode/Property.

So komisch sind die eigentlich garnicht. In C# hab ich solche Konstrukte schon haufenweise verwendet. Ohne Probleme.

Ich habe aber testweise die TDBUsers als ganz eigenständige Klasse gesetzt, was leider noch immer nichts an dem Problem geändert hat. GetDBUserByLogin() wird noch immer geflissentlich gemieden, obwohl explizit auf die zugehörige Property lesend zugegriffen wird :?. Daran lag es leider nicht. (Was heisst leider... das wäre recht dümmlich, wenn sowas nicht in Delphi ginge :))


EDIT: Okay, lustig...
Ich habe noch eine weitere Property "Users", die wie "UsersByLogin" arbeitet, aber eben eine ID statt eines Logins nimmt:

Delphi-Quellcode:

  TDB = class
  private
    class var FQuery: TUniQuery;
    class var FUser: TDBUser;
    class function GetDBUser(aUserID: Integer): TDBUser; static;
    class function GetDBUserByLogin(aLogin: string): TDBUser; static;
  public
    class property Users[aUserID: Integer]: TDBUser read GetDBUser;
    class property UsersByLogin[aLogin: string]: TDBUser read GetDBUserByLogin;
  end;

implementation

class function TDB.GetDBUser(aUserID: Integer): TDBUser;
begin
  FUser.FID := aUserID;
  result := FUser;
end;

Folgenden Methode:

Delphi-Quellcode:

procedure TframeUsers.btnResetPasswordClick(Sender: TObject);
var
  id: Integer;
begin
  id := qryUsers.FieldByName('id').AsInteger; // <----  !!!
  TDB.Users[id].Password := '';
end;

Hier wird mir vom Compiler gemeldet, dass der auf "id" zugewiesene Wert nicht mehr verwendet wird! :shock:

[implementation]

Zitat:

Zitat von Medium (Beitrag 1029749)

So komisch sind die eigentlich garnicht. In C# hab ich solche Konstrukte schon haufenweise verwendet. Ohne Probleme.

:shock: Welche VS-Version hast du? Also 2010 lässt es ganz sicher nicht zu, dass man in einer Public-Methode einen Private-Typ zurückliefert. Hab ich selber schon mehrmals versucht.

Ist ja auch total sinnlos. So verhinderst du ja, dass man den Rückgabewert in einer Variable speichern kann... Wat'n Schwachsinn...

[himitsu]

Eventuell schlägt sich auch hier wieder ein Fehler im Compiler nieder?

Versuch es mal so:

Delphi-Quellcode:

TDB = class
public
  type
    TDBUser = record
    private
      FID: Integer;
      // für jedes DB feld eben...
    public
      property ID: Integer read FID;
      property Name: string read FName;
    end;
private
  class var FQuery: TUniQuery;
  class var FUser: TDBUser;
  class function GetDBUserByLogin(aLogin: string): TDBUser; static;
public
  abc: Integer;  // dummies
  xyz: String;   //
  class property UsersByLogin[aLogin: string]: TDBUser read GetDBUserByLogin;
end;

Ansonsten wüßte ich auch nicht weiter (jedenfalls nicht ohne mal ein Testprojekt zum Testen zu haben).

[Medium]

@implementation:
#Develop. Und das mit dem Schwachsinn will ich mal dahin gestellt sein lassen. Es gibt hier schon einen triftigen Grund dafür, dass die Referenz selbst nicht gepeichert werden soll. Immerhin ist die gesamte Klasse quasi abstrakt - es werden ja gar keine Instanzen erzeugt.

Was ich damit schlicht erreichen will ist, dass man im eigentlichen Programm nachher die angenehme Schreibweise hat, ála:
TDB.Users[id].Name
TDB.Rights[id].Description
usw., und zwar je nach Feld nur lesend, oder auch schreibend. Dabei ist es ebenfalls wichtig, dass IMMER live aus der DB gelesen wird. Was will ich dann jemals mit einer Instanz von TDB.Users[id] sinnvoll anfangen wollen!?

@himi:
Ich bastels mal um. Edit: Leider kein Effekt. Die Properties sind in der eigentlichen (etwas größeren) Klasse auch nicht allein:

Delphi-Quellcode:

  public
    class function AddUser(aName, aSurname, aLogin, aPassword: string): Integer;
    class function UserExists(aUserID: Integer): Boolean; overload;
    class function UserExists(aLogin: string): Boolean; overload;
    class property Query: TUniQuery read FQuery write FQuery;
    class property Users[aUserID: Integer]: TDBUser read GetDBUser;
    class property UsersByLogin[aLogin: string]: TDBUser read GetDBUserByLogin;

[SirThornberry]

Das Problem mit den ganzen Class-Methoden ist das es nicht Threadsicher ist.
Wenn ein mehrere Threads gleichzeitig die Klasse verwenden kommen ganz "tolle" Ergebnisse dabei raus.
Was spricht dagegen eine ganze normale Klasse daraus zu machen und dann, wenn es unbedingt sein muss, eine "globale" Instanzvariable zu nutzen? Dann kannst du diese Bei Programmstart initialisieren und bei Programm Ende freigeben und wirst keine Probleme haben wenn mal mehrere Threads die Klasse zum gleichen Zeitpunkt verwenden.

Es geht also nicht darum alle Klassenmethoden zu entfernen sondern nur zu verhindern das Variablen von mehreren Threads gleichzeitig genutzt werden.

[Medium]

Hm, mal abgesehen davon, dass im gesamten Projekt bisher keine Verwendung von Threads vorgesehen ist, ließe sich im Zweifel das Zuweisen der FID und die Rückgabe der Klassenreferenz noch in eine Critical Section stopfen. Das Problem wäre mit einer globalen Instanzvariablen auch das selbe: Es würde die ID von einem Thread gesetzt, und er könnte sich schon nicht mehr sicher sein, dass sie von einem anderen nicht umgehend geändert wurde, noch bevor die 2 Dereferenzierungen bis zum gewünschten Ergebnis ausgeführt sind.
Oder meintest du die TDB-Klasse, nicht die Sub-Klassen*? Da könnte eine Instanz pro Thread noch Sinn machen, ich müsste mir dann nur grad überlegen ob und wo ich diese dann hin schmeissen will. Eigentlich wollte ich den Zugriff von überall im Programm möglich machen, ohne je etwas dafür Createn zu müssen.
*) Da wäre dann die Frage, ob jede TDB-Instanz eine ganz eigene TDBUser-Klassenreferenz hat, und nicht nachher mehrere TDBs auf die selbe TDBUser-Klasse zugreifen.

Da das hier offenbar alles zu nichts führt, frage ich einfach mal anders:
Wie würdet IHR es angehen, eine DB im Programm wie eine Collection aussehen zu lassen, die sämtliche Abfragen live aus der DB tätigt, und Zuweisungen auch umgehend in diese zurück speichert? Und zwar so, dass ich mir bei der Verwendung keinerlei Gedanken um Freigaben o.ä. machen muss.

[Medium]

Update: Wenn ich die inneren Klassen zu Records (mit normalen Methoden natürlich, aber nach wie vor private) mache, aber ansonsten nichts zur ersten Version ändere, DANN funktioniert das ganze! Ich würde das daher durchaus als tatsächlichen Bug bezeichnen, und da soweit keine weiteren Vorschläge zum grundsätzlichen Vorgehen kamen, bzw. ich mit dem hier eigentlich recht zufireden bin, kann ich das Thema denke ich als gelöst betrachten.
Edit: Und das sollte auch die Problematik mit der Threadsicherheit eliminieren, da ja jetzt immer ein neuer Record geliefert werden dürfte.

Nochmals vielen Dank an alle!

[SirThornberry]

genau, damit sollte es auch Threadsicher sein.
Ich hatte das Global in Anführungszeichen gepackt weil ich nicht wirklich global meinte sondern global innerhalb einer Threadklasse, innerhalb deiner Klasse die das handelt etc. so das aber keiner von außen drauf zugreifen kann. Aber durch den Umstieg auf Records hat sich das erübrigt.

[Medium]

Jau, das hatte ich auch so verstanden. Meine Bauchschmerzen diesbezüglich lagen auch beim Hauptthread, dem ich mehr als ungerne eine unitübergreifende Magic-Super-Global spendiert hätte. Deswegen hab ich ja als aller erste Anlaufstelle die Klasse TDB, die nur statische Elemente enthält, damit man eben nicht nachher nicht so recht weiss wohin mit der Instanz, bzw. sich überhaupt ums Erzeugen/Freigeben kümmern muss.

Ziel ist hierbei übrigens die totale Entkopplung der Logik von der Datenhaltung, so weit, dass im eigentlichen Programm kein Fitzel SQL mehr auftaucht. Wenn die DB getauscht würde, müssten in der TDB-Klasse lediglich 6 Strings (SQL-Prototypen quasi*) angepasst werden falls sich an der Syntax was tut, und eben das Query-Feld gegen die entsprechende neue Zugriffskomponente getauscht werden. Im Zugriff programmseitig schaut das nachher wie eine große Collection aus, die aber immer "frisch" ist, und keinerlei Polling o.ä. bedarf.
*) Gut, es gibt auch eine Hand voll speziellerer und umfangreicherer Statements, die nicht aus den Prototypen gebildet werden, und daher auch ggf. anstünden. Aber dafür jeder auch wirklich nur ein Mal.

Ich hätte auch ehrlich gesagt angenommen, dass das Vorgehen im Groben recht üblich ist :)