fau-fablab / FabLabKasse

FabLabKasse, a Point-of-Sale Software for FabLabs and other public and trust-based workshops
https://fablabkasse.readthedocs.io
GNU General Public License v3.0
15 stars 4 forks source link

kassenbuch: Performance gains/Database-interface #139

Open schrolli opened 8 years ago

schrolli commented 8 years ago

Um die Diskussion, was man noch alles machen könnte, vom pull request #138 wegzuziehen, hier ein seperates Issue, um Ideen zu besprechen, die die geschwindigkeit steigern können.

Ich bin auch gewillt das in ein paar pull-requests zu verpacken, wollte vorher nur eure meinung hören, ob ihr das sinnvoll findet oder es layout-technisch blöd ist. Hier also die Sachen, die mir aufgefallen sind:

Caching

Ein paar Sachen, bei denen ein kleines Caching vll etwas bringen könnte:

Rechnung.summe oder Kunde.summe ist ein property, das jedes mal, wenn auf dieses zugegriffen wird über alle Positionen/Buchungen drübercrawled. Wäre da kein internes Attribut besser, das man beim ersten Aufruf schreibt und bei konsekutiven Aufrufen zurückgegeben wird? Wenn sich etwas an den Positionen/Buchungen ändert, wird das interne Attribut auf None gesetzt und beim nächsten Zugriff wieder neu berechnet.

Das gleiche könnte man auch mit Kasse.buchungen, Kasse.rechnungen und Kasse.kunden machen.

Rechnung.summe_position kann man im sql-abruf von _load_positionen machen: SELECT anzahl, einzelpreis, anzahl * einzelpreis AS summe. Und das dann auch mit in das dict abspeichern. die berechnung muss dann halt in python in der add_position auch noch gemacht werden. Sonst werden ja die Positionen nicht verändert, oder?

Benennung/Funktion

Rechnung.store und Buchung._store legen einfach eine neue kopie an, auch wenn das aktuelle objekt schon in der datenbank steht??? ist das dann eine "Kopierfunktion"? dann würde ich die Funktion aber irgendwie in store_as_new umbenennen... Kunde.store und Kundenbuchung.store machen das "richtig"...

Abfrage der zugeordneten Elemente/in sql-abfragen per Name auf Spalten zugreifen

z.b. in Kasse.get_buchungen() wird (auch noch nach unseren Verbesserungen) nur die ID der Buchung abgefragt und dann Buchung.load_from_id() übergeben.

Ich sehe aktuell die Notwendigkeit, dies zu kapseln. man muss auf die einzelnen spalten per index zugreifen und will nicht überall SELECT id, datum, konto, rechnung, betrag, kommentar FROM buchung hinschreiben und wenn sich was ändert überall nachbessern müssen.

Den Zugriff per Indizes find ich sowieso ungut. Früher hab ich das in php und mysql aber auch gemacht. Mit Namen referenziert liest sich der Code aber viel besser... Für sqlite3 macht das row_factory attribut einen namens-basierten zugriff möglich:

https://docs.python.org/2/library/sqlite3.html#accessing-columns-by-name-instead-of-by-index

das macht erstmal den zugriff mit Indizes nicht kaputt, erlaubt aber trotzdem den Zugriff per spalten-name.

Man könnte also in Kasse.get_buchungen Select * from buchungen where blablabla machen und dann für jede zeile Buchung.load_from_row aufrufen. Die muss dann natürlich die spalten mit namen referenzieren. Dadurch hat man nur eine einzige SQL-Abfrage und nicht für n Zeilen n+1 Abfragen

das gleiche bei Kasse.get_rechnungen und Kasse.kunden

todos

sedrubal commented 7 years ago

https://github.com/fau-fablab/FabLabKasse/pull/138#issuecomment-224939793

schrolli commented 7 years ago

also das ganze gecache in der Datenbank würde ich eher damit lösen, dass man eine gescheite Datenbank-Engine verwendet. Wir hatten das doch damals diskutiert. sqlite ist dafür einfach viel zu dumm. mysql hätte z.b. einen Festkomma-Zahlentyp und kann auch direkt damit rechnen. Dann bräuchte man auch keinen Cache mehr, da man einfach alle Summier-Aufgaben in die Datenbank auslagern könnte. Die oben angesprochenen Punkte könnte man auch noch ohne Umstieg auf eine andere DB-Engine machen. Ist aber auch nur rumdoktorn und löst das eigentliche Problem nicht... Da sich hier aber auch noch keiner über die Vorschläge geäußert hat, hab ich da noch nichts gemacht.

sedrubal commented 7 years ago

Achso. Julian war nur gerade da und meinte, dass man es so machen könnte. Ich hab die Diskussionen damals nicht mit verfolgt.

schrolli commented 7 years ago

Nach nochmaligen drüber nachdenken jetzt, meine ich, dass das caching von Summen in einer extra tabelle wsl. nicht so viel bringt, als z.b. das abfragen aller Buchungen in einer einzigen sql-Abfrage. Bei dem Cache, da dann an zwei verschiedenen orten die "selbe" information gespeichert ist, kann auch recht schnell was korrupt werden... Da ich ja damals schon mitm Patrick recht viel Performance durch solche Umstrukturierungen (python-objekt rumgeschubse -> sql-query) rausgeholt habe, denke ich, dass das der bessere weg ist, als sich eine caching-sache ins haus zu holen... @sedrubal das oben sollte da auch kein Seitenhieb an dich sein o.ä. Ich wollte meine Meinung dazu nur nochmal kund tun;)

@all: sagt mal, was ihr von den oben angesprochenen Veränderungen haltet. Wenn nur keiner was dazu sagt, weiss ich halt auch nicht, ob das jetzt dann gewünscht wird oder einfach ignoriert...