Hallöle...

Ich lehne mich mal weit aus dem Fenster ohne alles nachgelesen zu haben...

[Nur meine Meinung...über vieles läßt sich streiten ]
1. Hat das einen Sinn das verschiede Variablen den_ Unterstrich tragen statt wie "üblich" F?
2. Warum hat das Event für den Thread den Namen "TThreadBoolean" statt einem sprechenden Namen wie "TFinishLoadList" ... am Namen sollte man schon die Funktionsweise "erahnen"
3. Die Connection gehört in den Thread...
4. Das gehört nicht in den Thread thrReadDb.ReadyEvent := Form2.SyncReady; Den Eventhander in der Form deklarieren und verbinden.
5. TListData = class(TList<TValues>) ... erzeugt dir Speicherlecks mit Values := TValues.Create; besser http://docwiki.embarcadero.com/Libra...ectList.Create
6. Keine Passworte hardcodiert im Connectionstring
7. thrReadDb.Resume; ... nicht mehr verwenden sondern START
8. thrReadDb.FreeOnTerminate := TRUE; habe ich fast überlesen zwischen den Kommentaren.
9. thrReadDb.dbcon.ProviderName := TSQLServerUniProvider.Create(NIL).Name; ... einfach zuordnen statt von neuer Instanz. (imho legt die UniDAC automatisch intern eine Instanz an) besser: FConnection.ProviderName := 'Interbase';
10. Aufteilung in separate Units. Die Logik (Thread) hat nichts in der Form Unit zu suchen.
11. WICHTG: Der Code ist nicht konsistent. Mal "true" mal "TRUE". Benutze lieber einen Codeformatter...

Hallo,

vielen Danke fürs Feedback. Ich weiß dass es mühsam ist, den ganzen Code anzuschauen. Wenn ich aber 10 Threads mit unterschiedlichen Fragen öffne wirds wohl auch nicht einfacher. Deshalb habe ich mal alles in einen Topf geworfen (und auch in eine Unit )

Zitat von haentschman:

1. Hat das eine Sinn das verschiede Variablen den_ Unterstrich tragen statt wie "üblich" F?

Habe ich geändert. Kam aus einem der ersten Beispiele und ich dachte das macht man bei Threads vieleicht so

Zitat von haentschman:

2. Warum hat das Event für den Thread den Namen "TThreadBoolean" statt einem sprechenden Namen wie "TFinishLoadList" ... am Namen sollte man schon die Funktionsweise "erahnen"

Weil ich den für verschiedene Aufgaben verwende wo ich ein Boolean übergebe

Zitat von haentschman:

4. Das gehört nicht in den Thread thrReadDb.ReadyEvent := Form2.SyncReady; Den Eventhander in der Form deklarieren und verbinden.

Habe ich geändert als ich den Thread in eine eigen Unit verschoben habe und eine Rückverlinkung zum Form benötigt hätte

Zitat von haentschman:

5. TListData = class(TList<TValues>) ... erzeugt dir Speicherlecks mit Values := TValues.Create; besser http://docwiki.embarcadero.com/Libra...ectList.Create

Habe ich mir angeschaut. Hintergrung für TListData = class(TList<TValues>) ist, dass ich die Liste dem Master-Record anhängen will. In etwas so:

markieren

Delphi-Quellcode:

  TCalReport = class
  private
    FNr : String;
    ...
    FValues: TListData
  public
    ...
  end;

Zitat von haentschman:

6. Keine Passworte hardcodiert im Connectionstring

War nur für die Schnelle

Zitat von haentschman:

7. thrReadDb.Resume; ... nicht mehr verwenden sondern START

Werde ich ändern

Zitat von haentschman:

8. thrReadDb.FreeOnTerminate := TRUE; habe ich fast überlesen zwischen den Kommentaren.

Was meinst du damit?

Zitat von haentschman:

9. thrReadDb.dbcon.ProviderName := TSQLServerUniProvider.Create(NIL).Name; ... einfach zuordnen statt von neuer Instanz. (imho legt die UniDAC automatisch intern eine Instanz an) besser: FConnection.ProviderName := 'Interbase';

Habe die DB-Komponenten bisher immer in einem Datamodul angelegt. Da muss ich wohl noch üben.

Zitat von haentschman:

10. Aufteilung in separate Units. Die Logik (Thread) hat nichts in der Form Unit zu suchen.

Da war ich die letzten Stunden dran, denn das ist ja eines der Ziele der Übung. Trennung GUI - Logic - Daten

Zitat von haentschman:

11. WICHTG: Der Code ist nicht konsistent. Mal "true" mal "TRUE". Benutze lieber einen Codeformatter...

Bin ich mir am angewöhnen.

Es ist nicht ganz einfach Quellen zu finden, auf die man sich verlassen kann. Auch die Delphi-Beispiele verwenden z.B. Resume http://docwiki.embarcadero.com/CodeE...dList_(Delphi)
Und im Moment habe ich noch meine liebe Mühe mit dem Thema "Thread"

Grüße
Gerd

Hallöle...

Zitat:

8. thrReadDb.FreeOnTerminate := TRUE; habe ich fast überlesen zwischen den Kommentaren.
Was meinst du damit?

Das ist ja in Ordnung so. Ich hätte nur das thrReadDb.FreeOnTerminate := TRUE; direkt nach dem Erzeugen plaziert und die Kommentare weggelassen. Am Anfang bin ich davon ausgegangen das der Tread nicht fregegeben wird.

Zitat:

Habe ich mir angeschaut. Hintergrung für TListData = class(TList<TValues>) ist, dass ich die Liste dem Master-Record anhängen will. In etwas so:

Auch wenn der Master Record freigeben wird, gilt das nicht für den Listeninhalt. Du legst eine Instanz rein Values := TValues.Create; und die sollte freigegeben werden. Der Master Record kümmert sich nur um die Listeninstanz. Den tangiert in keinster Weise der Inhalt der Liste...

Zitat:

2. Warum hat das Event für den Thread den Namen "TThreadBoolean" statt einem sprechenden Namen wie "TFinishLoadList" ... am Namen sollte man schon die Funktionsweise "erahnen"

Über Namen kann man streiten. Ist ja in Ordnung so. Was ich meinte... Lieber ein Event mehr deklariert als ein "Master" Event. Bei dem Beispiel mit der Liste kann man im besser genahmsten Event die Liste im Event mit übergeben. Da gibt es zig Varianten der Ausführung...