Kundenliste

[EdAdvokat]

Habe mir nun die procedure SaveToDB vorgenommen. Wenn ich sie aufrufe speichert er unendlich lange und die Datenbank wird immer größer. Er addiert vermutlich die besthenden Datensätze bis ich das Programm gewaltsam abbreche. Habe dann gesondert nur Insert mit den entsprechenden Werten aufgerufen, mit dem gleichen Ergebnis. Zwischenzeitlich wieder eine frühere Version der DB eingespielt mit weniger Datensätzen. Auch wenn ich nur mit Update arbeite das gleiche Ergebnis, jedoch wird der neu erstellte Datensatz nicht gespeichert.
Die for i:=0 to self.count-1 do Schleife ist doch notwenig. Er holt sich doch damit die Daten aus der Objectlist und speichert sie in der Datenbank - oder? Er sollte doch bei self.count-1 aufhören.
Mit dem Versuch den Debugger anzuwerfen bin ich gescheitert. Habe auf den Beginn der Procedure SaveToDB einen Haltepunkt gesetzt und dann den Button gedrückt und das Drama nahm seinen Lauf. Er speichert und speichert bis ich ihn abwürge.Natürlich kein Ergebnis dieses Versuches. Aber vielleicht mache ich das auch falsch. Jedenfalls glaube ich noch immer dass es doch klappen sollte. Ich sehe jedoch keine weiteren Stellschrauben in diesr procedure, nachdem ich alles schon gesondert probiert habe.
Ist die Procedure wirklich richtig?
Zwischenzeitlich habe ich mir den Programmablauf auf Zettel kommentiert, um genau nachzuvollziehen was wann passiert. Leider noch nicht abschließend.

[Hobbycoder]

also das speichern geht bei mir wunderbar. Wenn gleich es auch sehr lange dauert.

Generell ist es natürlich besser, die veränderten Daten gleich zu speichern. Du brauchst lediglich zu dem Object TCustomer zwei Methoden zum Speichern und zum Löschen hinzufügen. Beim Speichern wieder die ID rüfen, wenn =-1 dann insert sonst update. Und in der Mainform rufst du die dann bei den Buttons passen auf.

Wenn du aber bei deiner bisherigen Form bleiben willst, dann setzt die bei gelöschten Datensätzen die ID einfach auf -2. dann kannst du in der SaveToDB auf id=-2 prüfen und statt insert bzw. update machst du dir noch einen Remove-Statement dazu. Allerdings müsstest du dann dort auch das dazugehörige Object löschen, damit du hinterher auch mit FuelleListView die Anzeige neu aufbauen kannst. Und du müsstest in der SaveToDB entweder for i:=self.count-1 downto 0 machen, oder alle Object die ID=-2 habe erst nach dem Speichern und vor dem FuelleListview löschen.

Klinkt umständlich...ist es auch. Besser wie oben und gleich speichern, löschen und hinzufügen, und danach immer FuelleListView. Das hätte auch den Vorteil, dass dein Programm auch mit mehreren Clients funktionieren könnte. Denn zur Zeit würde ein Speichern der DB immer die Änderungen überschreiben, die ein anderer User eventuell gerade gespeichert hat, oder sogar zu Datenverlust führen.
Stell dir mal vor, du und ein anderer User laden die DB an 2 Rechnern. Jetzt löscht der andere einen Datensatz und speichert.
Anschließend änderst du den gleichen Datensatz. bei dir ist ja noch in der CustomerList vorhanden. Anschließend willst du speichern. Dein SQL-Statement UPDATE kann diesen Datensatz aber nicht finden (where ID=:id) und speichert ihn dem zur Folge auch nicht. Weg isser. Und so ließen sich noch weitere Szenarien darstellen.

[EdAdvokat]

Ich bin da erst einmal ratlos. Man kann es drehen und wenden, die procedure SaveToDB stimmt irgenwie nicht. Weder mit  for I:= Self.Count-1 downto 0 do noch mit  for I:=0 to Self.count-1 do läuft die Sache rund.
Es ist doch nicht normal, dass ein Speichervorgang von ca. 1000 Datensätzen gut 5 Minuten dauert und im Ergebnis dann 2000 Datensätze vorhanden sind, obwohl überhaupt keine Veränderung in der Datenbank vorgenommen wurde.
Das Hinzufügen einer Methode Speichern zum Objekt TCustumer würde nach meinem Verständnis bedeuten, dass ich die gleiche Procedure SaveToDB von TCustomerList hin zu TCustomer schreibe und dann aufrufen soll?
Das wäre doch das gleiche in Grün.
Möglicherweise habe ich ein Brett vor dem Kopf. Ich vermute, dass die SaveToDB-procedure eine Macke hat, egal ob for I:= 0 to self.count-1 do oder umgekehrt. Es ist doch nicht normal, dass ein solcher Vorgang gut 6 Minuten dauern soll.
Leider bin ich mit meinem Latein am Ende.

[p80286]

Zitat von EdAdvokat:

Es ist doch nicht normal, dass ein Speichervorgang von ca. 1000 Datensätzen gut 5 Minuten dauert und im Ergebnis dann 2000 Datensätze vorhanden sind, obwohl überhaupt keine Veränderung in der Datenbank vorgenommen wurde.

Wenn 2000 Datensätze vorhanden sind, und Du 1000 Datensätze gespeichert hast, dann waren wohl 1000 Datensätze vorhanden. Waren keine Datensätze vorhanden, ist vllt. jeder Datensatz zwei mal gespeichert worden?
Keine Veränderung in der Datenbank? also wurde nichts gespeichert und die 2000 Datensätze waren von Anfang an vorhanden?

Was die Speicherdauer angeht, das kommt ganz auf den Server und die gespeicherte Datenmenge an, aber 5 Minuten ist schon recht happig.

Gruß
K-H

[Hobbycoder]

Du hättest als erstes mal die Möglichkeit einen Haltepunkt am Anfang der SaveToDB-Routine zu setzen, und mit F8 mal schrittweise durchzusteppen. Dann schaust du dir erst mal den Wert von CustomerList.Count an, ob der überhaupt bei 2000 liegt, bzw. sich jedes mal verdoppelt.
Dann würde ich mal einen Blick auf die ID werfen. Wenn die >-1 ist, springt er auf das richtige SQL-Statement (Update)? Oder macht er womöglich immer ein insert?

Du kannst auch mal folgendes Probieren. Setze eine Haltepunkt auf die Zeile zqryMain.Params.ParamValues['ID']:=self[i].ID; . Diese wird ja nur bei einem Insert aufgerufen. Dann Programm starten, DB Laden und direkt danach wieder speichern. Da kein Datensatz neu ist, dürfte ja kein Insert auftreten und die Zeile nie angesprochen werden. Sollte das doch der Fall sein, stimmt ja was mit der ID nicht.

Weiterhin könntest du noch mal die Routine LoadFromDB anschauen. Werden da wirklich vor dem Laden alle Object von der CustomerList gelöscht (CustomerList.Clear)? Wenn nicht, würde das bedeuten, dass er immer alles Datensätze beim laden anhängt, und dann hättest du nach dem Laden immer 1000 mehr (Sofern 1000 in der DB sind).

Warum überhaupt gleich 1000 Datensätze zum Testen/Debuggen? Das macht das ganze doch eher unübersichtlich und langwierig. Fang mit einem Datensatz an. Laden, Editieren, Speicher und wieder Laden. Sind es dann 2? Auch das löschen und Hinzufügen zum testen wäre mit weniger Datensätzen schneller erledigt. Im Normalfall gilt, was mit 10 Datensätzen fehlerfrei läuft, läuft auch mit 1000 und 10000.
Vielleicht baust du dir auch eine Statuszeile ein, in der die CustomerList.Count steht, und nach jeder Aktion aktualisiert wird.

Aber damit du nicht ewig suchst, dein Fehler in der SaveToDB. Genauer beim zqryMain.sql.text:=. Wenn ID=-1 dann lädst du das Update und wenn ID>-1 dann verwendest du Insert. Das ist logischerweise verkehr herum. Beim Bestücken der Parameter machst du es dann aber wieder richtig.
Aber du hast noch andere Fehler drin, die dir später noch viel Kopfzerbrechen bereiten werden.

[Hobbycoder]

Ich habe mal ein bisschen rumgespielt, und mir mal eine Zeitmessung eingebaut.

Bei 47416 Datensätzen dauert
das Laden aus der DB: 17 Sek.
das Füllen des Listview: 232 Sek.
das Speichern (mit deinem Fehler): 398 Sek. (weil ja fehlerhafterweise jeder Datensatz mit Insert neu eingefügt wird).

Damit könnte keiner arbeiten. Selbst wenn du den Fehler beseitigst, wird die Zeit für das Speichern nicht so kleiner, dass es komfortabel wäre, denn es würde ja auch jeder Datensatz trotzdem gespeichert, nur halt mit Update.

Abhilfe, bei jetzigem Programmaufbau, würde z.B. eine Hilfevariable im Customer-Object schaffen. Du könnte in der TCustomer-Klasse noch die Property Modified als Boolean einfügen, die im Constructor Create erst einmal auf False gesetzt wird.
Wird nun ein Datensatz verändert, oder hinzugefügt, so wird CustomerList[x].Modified bzw. Customer.Modified auf True gesetzt.

In der SaveToDB-Routine könntest du nun erst einmal auf Modified prüfen und so entscheiden, ob der Datensatz überhaupt gespeichert werden muss. So hättest du die Zeit für das Speichern auf wenige Sekunden oder Millisekunden reduziert. Darfst aber nicht vergessen, nach dem Speichern des Datensatzes Modified wieder auf False zu setzen.

Sinnvoll wäre auch, nach dem editieren den zugehörigen ListView-Eintrag gleich zu aktualisieren, damit nach dem SaveToDB auch wirklich das richtige im Listview steht (ist natürlich suboptimal, aber bei jetzigem Programmaufbau am sinnvollsten).

Ähnliches Problem besteht beim löschen eines Datensatzes. Zur Zeit löscht du ihn nur aus der CustomerList und machst anschließend ein FuelleListView. Damit ist er zwar aus der Liste, aber in der DB immer noch vorhanden und beim nächsten Laden wieder da.
Außerdem dauert das FuelleListView bei 47416 Datensätzen ja fast 4 Minuten. Also inakzeptabel.
Hier hättest du 2 Möglichkeiten, entweder du entfernst per SQL den Datensatz gleich aus der DB und den dazugehörigen Eintrag aus der ListView, oder du machst es wie oben mit einer Hilfsproperty, die z.B. Deleted heißen könnte. Auf die müsstest du dann aber in der SaveToDB auch eingehen, und ein passendes Delete-Statement für SQL bereitstellen.

Was die Zeit angeht, die das Listview benötigt, bis alles Daten der CustomerList dort vorhanden ist, so wird man dort wohl keine signifikante Geschwindigkeitssteigerung erreichen. Zumindest nicht mit Möglichkeiten, die du jetzt hier auch verstehen würdest. Für diesen Fall würde sich aber ein VirtualStringTree gerade zu aufdrängen. Das ist zwar vom Handling her etwas komplizierter, aber wenn man die Arbeitsweise erst mal verstanden hat, schon fast genial. Und in deinem Fall gut einsetzbar, da du bereit die passenden Objecte vorliegen hast.

Und für alle anderen, die das jetzt gelesen haben: Sicherlich gibt es wesentlich bessere Möglichkeiten das speichern der Daten so zu realisieren, damit viele der oben genannten Schritte gar nicht notwendig wären. Aber ich wollte EdAdvokat erst mal nur die Möglichkeiten und Probleme aufzeigen, ohne Ihn vollends zu verwirren.

[EdAdvokat]

Vielen Dank für die vielen Hinweise, die ich z.T. jedoch noch nicht verarbeiten kann. Ich konzentriere mich zunächst darauf die SaveToDB-procedure ordentlich zum laufen zu bekommen und dann werde ich mich an die Hinweise zur Verbsserung des Programms machen. Meine Versuche mit dem Debugger zu arbeiten waren durchwachsen. Ich habe den Hinweis mit dem Haltepunkt beachtet und mit F8 Zeile für Zeile geprüft. Er durchlief tatsächlich nur die update-Routine und nicht Insert. Die Prüfung in der LoadFromDB ob clear tatsächlich keinen Datensatz mehr hat war für mich sehr unübersichtlich, denn ich weiss gar nicht was ich da wo suchen sollte.
Nun erst einmal zum Ergebnis SaveToDB hier die jetzige Fassung, hoffentlich richtig:

Delphi-Quellcode:

procedure TCustomerList.SavetoDB(con: TZConnection);
var
  zqryMain: TZQuery;
  i: Integer;
begin
  zqryMain:=TZQuery.Create(nil);
  try
    zqryMain.connection:=con;

for I:= 0 to self.Count-1 do
begin
   if self[i].ID>-1 then
     zqryMain.SQL.Text:='UPDATE WARENVERKAUF1 SET KDNR=:KNR, NAME =:NAM, VORNAME=:VNA, FIRMA=:FIR, PRODUKT=:PRO, ANZAHL=:ANZ, PREIS=:PRE WHERE ID=:CID' else
     zqryMain.SQL.text:='INSERT INTO WARENVERKAUF1(KDNR,NAME,VORNAME,FIRMA,PRODUKT,ANZAHL,PREIS) VALUES(:KNR, :NAM, :VNA, :FIR, :PRO, :ANZ, :PRE)';
     zqryMain.params.parseSQL(zqryMain.sql.text, True);
   if self[i].ID>-1 then
   begin
     zqryMain.Params.ParamValues['CID']:=self[i].ID; //update
     zqryMain.params.ParamValues['KNR']:=self[i].KDNR;
     zqryMain.params.paramValues['NAM']:=self[i].Name;
     zqryMain.params.paramValues['VNA']:=self[i].Vorname;
     zqryMain.params.ParamValues['FIR']:=self[i].Firma;
     zqryMain.Params.ParamValues['PRO']:=self[i].Produkt;
     zqryMain.Params.ParamValues['ANZ']:=self[i].Anzahl;
     zqryMain.params.paramValues['PRE']:=self[i].Preis;
     zqryMain.ExecSQL;
   end else
   begin
     zqryMain.params.ParamValues['KNR']:=self[i].KDNR; //insert
     zqryMain.params.paramValues['NAM']:=self[i].Name;
     zqryMain.params.paramValues['VNA']:=self[i].Vorname;
     zqryMain.params.ParamValues['FIR']:=self[i].Firma;
     zqryMain.Params.ParamValues['PRO']:=self[i].Produkt;
     zqryMain.Params.ParamValues['ANZ']:=self[i].Anzahl;
     zqryMain.params.paramValues['PRE']:=self[i].Preis;
     zqryMain.ExecSQL;
   end;
end;

  finally
    zqryMain.Free;
  end;
end;

Damit kann ich einen hinzugefügten Datensatz(der als ID zunächst -1 hat) speichern und dann wieder laden und er hat dann eine ID der Datenbank, so wie es sein sollte. Auch das Bearbeiten eines bestehenden Datensatzes und das Speichern klappt. Nach Laden mit LoadFromDB ist der geänderte Datensatz sichtbar. Nur nach dem Löschen von Datensätzen und Speichern erscheint nach LoadFromDB oder Neustart des Programms wieder der gelöschte Datensatz. Das bekomme ich einfach nicht hin.
Ich hätte noch so viele Fragen, aber zunächst soll das Hauptproblem gelöst werden. Ist die Procedure wirklich richtig????
Im Hauptformular rufe ich mit dem Button löschen die Routine auf: CustomerList.Delete(lvCustomer.Selected.Index); hier wird doch der ausgewählte Datensatz aus der objectlist gelöscht. Danach wird fülleListview aufgerufen. Das ist doch auch i.o.
Wenn ich wüßte wie ich mit dem Debugger diese Problematik prüfen könnte. Da er ja tatsächlich auch sichtbar den Datensatz aus der Liste löscht, jedoch beim Speichern ihn wohl nicht gelöscht speichert, erscheint er danach wieder.