Verständnisfrage: Wem übergebe ich die Argumente, der Klasse oder der Methode?

[Sophus]

Hallo Leute,

ich gebe zu, der Betreff dieses Themas ist nicht ganz getroffen. Eines vorweg, ehe wir zum Anliegen kommen. Ich möchte, dass wir uns die leidige Kritik wie "Name der Methode ist nicht zutreffend" oder "Name der Klasse ist doof" oder "Das Beispiel ist einfach nicht sinnvoll" etc. Die angeführten Beispiele sollen nur den eigentlichen Kern des Themas erfassen. Aus diesem Grund habe ich mir keine Gedanken über den Sinn oder Unsinn der Beispiele gemacht. Ersparen wir es uns also.

Kommen wir zum Verständnisproblem. Ich arbeite in meinem Projekt mit recht vielen Klassen. Und ich erwische mich immer wieder, dass ich jede Klasse anders behandle. Es passiert oft, dass ich der instantisierten Klasse A kaum Argumente übergebe, dafür mehr der jeweiligen Methoden. Dann passiert auch, dass ich der instantisierten Klasse B recht viele Argumente übergebe, und die jeweiligen Methoden recht wenig. Nun meine Frage vorab: Ab wann übergebt ihr der instantisierten Klasse Argumente und ab wann der jeweiligen Methode? Ich habe für euch zwei Beispiele mitgebracht, und werde im Anschluss mein Bauchgefühl äußern.

Code: Alles auswählen

from PyQt4.QtCore import QObject

class CalculatorVersionOne(QObject):
    def __init__(self,
                 parent = None):
        QObject.__init__(self, parent)

    def calculate_bmi(self,
                      bodyweight,
                      bodyheight):
        
        return round(bodyweight / ((bodyheight/100) ** 2))

class CalculatorVersionTwo(QObject):
    def __init__(self,
                 parent = None,
                 **kwargs):
        QObject.__init__(self, parent)

        self.bodyweight = kwargs.get('bodyweight')
        self.bodyheight = kwargs.get('bodyheight')

    def calculate_bmi(self):
        
        return round(self.bodyweight / ((self.bodyheight/100) ** 2))


calculator_version_one = CalculatorVersionOne()
bmi_result_v1 = calculator_version_one.calculate_bmi(bodyweight = 85,
                                                  bodyheight = 184)

print "Result BMI V1", bmi_result_v1

calculator_version_two = CalculatorVersionTwo(bodyweight = 85,
                                              bodyheight = 184)

print "Result BMI V2", calculator_version_two.calculate_bmi()

Was wir hier sehen liegt auf der Hand. Ich habe zwei Klassen-Versionen, die jeweils eine Methode hat. Wie unschwer zu erkennen ist, geht es hier um BMI-Berechnung. Mir fiel auf die Schnelle keine bessere und schlichtere Berechnung ein. Gut, kommen wir zu meinem Bauchgefühl. In Version 1 wird der instantisierten Klasse kein Argument übergeben, sonder lediglich der Methode. Vorteil aus meiner Sicht: Die Methode lässt sich wunderbar testen. Selbst wenn die Klasse überaus komplex wäre, würde ich mir die Frage stellen, ob ich die Methode nicht eher zur Funktion umwandeln würde? Allerdings ist es bei mir recht mühselig, eine Methode aus einer Klasse rauszureißen und diese dann in eine Funktion umzuwandeln. Und in Version 2 übergebe ich der instantisierten Klasse alle Argumente, und rufe auf der Instanz die entsprechende Methode auf. Diese Version erscheint mir auf dem ersten Blick "pythonischer", da hier die Daten gekapselt sind. Das "Geheimnis-Prinzip" bleibt hier gewahrt. Allerdings ist die Methode schlecht testbar, da diese Methode klassenweit auf die Attribute zugreift. Aus diesem Grund wirkt es eher unsauber auf mich. Allerdings gibt es auch Klassen auf anderen Modulen von Drittanbietern, bei denen es auch vorkommt, dass man dort zunächst eine Klasse instantisiert und dann nur einer bestimmten Methode Argumente übergibt.

[__deets__]

Es ist alles andere als pythonisch, Daten aus Prinzip in Objekte zu kapseln. Es gibt Sprachen wie Java, bei denen das durch deren Sprachdesign fast schon erzwungen wird. Aber Python kennt Funktionen als gleichberechtigte Buerger.

Fuer dein praesentiertes Problem ist die Antwort darum ganz klar: Version 3, nur eine Funktion, welche die Berechung in Abhaengigkeit ihrere Argumente durchfuehrt.

In einem allgemeineren Kontext sollte man sich ein Gebot der Zustandssparsamkeit auferlegen. Nicht erst seit der DSVGO. Das heisst: so viele Argumente wie moeglich an Methoden uebergeben, *wenn* diese sinnvoll aus dem umgebenden Kontext zu erhalten sind. Und so wenig Zustand im Objekt wie moeglich.

Wann immer du so etwas siehst:

Code: Alles auswählen

foo = EinObjekt(mit, parametern)
ergebnis = foo.tuwas()
# und jetzt wird foo nie wieder gebraucht und in naher Zukunft garbage collected

siehst du schlechten Code vor dir. Der sollte eine Funktion sein.

[Sophus]

@__deets__: Ohne es jetzt komplizierter zu machen. Ich arbeite viel mit Klassen, im Kontext von QThreads. Das heißt, die Klasse, die dann später Thread-affin werden, kommunizieren mit der Datenbank. Stell dir vor, ich habe in der instantisierten Klasse A eine Methode namens add_reacord(). Würdest du der Methode die Daten übergeben, die abgespeichert werden soll, oder der instantisierten Klasse, die nachher Thread-affin wird? Beim Thread-Start sagst du ja dann, welche Methode dann gestartet werden soll (meistens ist es die run()-Methode). Demnach ist es doch Geschmacksfrage, oder?

[__deets__]

Ohne deinen Code zu kennen wuerde ich behaupten: ja, ich wuerde eine Methode mit allen Argumenten machen. Man macht einen Worker-Thread, und der sollte langlebig sein, und immer wieder Arbeitsauftraege entgegen nehmen, die per Queue aus eben einer Methode mit X Argumenten reinkommen. Oder bei Qt halt eine queued signal/slot connection. Damit sind sowohl signal als auch slot Zustandslos. Genau so ist das auch bei Qt gedacht, weshalb man den Thread ja auch nicht mit run ueberschreibt, sondern ihn seine eigene Ereignisschleife abarbeiten laesst.

So wie das bei dir klingt, hast du stattdessen eine Klasse, der du Argumente uebergibst, die bindet die brav an self, dann feuert sie selbst einen Thread hoch und in dessen run-Methode wertest du das dann aus. Das ist ein ganz schlechtes Muster, denn wenn ich so eine Klasse mal kurz 1000 mal aufrufe, dann hat man 1000 mal den Overhead einen Thread aufzubauen, um dann genau einmal was zu machen, statt einmal einen Thread zu erzeugen, und den dann tausend mal arbeiten zu lassen. Damit belastest du dein System ueber die Gebuehr, und gewinnst nichts.

[Sophus]

@__deets__: Ich fürchte, dass deine Annahme falsch ist. Mein Quelltext ist enorm lang, und sprengt hier den Rahmen. Und ehrlich gesagt, bin ich gerade zu faul, daraus ein lauffähiges Mini-Beispiel zu basteln, welches dann mein Programm abbildet. Daher zeige ich dir mal ein Pseudo-Quelltext, der nicht lauffähig ist.

Code: Alles auswählen

class MainThread(QDialog):
    def __init__(self,
                 parent = None,
                 **kwargs):
        QObject.__init__(self, parent)

        #[...]

    def start_task_thread(self):
        #[...]
        
        self.task_thread = QThread()

        self.task_thread.work = WorkerThread(arg1 = 'foo',
                                             arg2 = 'bar')

        self.task_thread.work.moveToThread(self.task_thread)

        self.task_thread.finished.connect(self.task_thread.deleteLater)

        self.task_thread.started.connect(add_record)
        
        self.task_thread.start()
        
        #[...]
        

An diesem Peudo-Programm zeige ich dir, wie ich mich oft erwische. Hier übergebe ich einem WorkerThread() die abzuspeichernden Daten, starte dann den Thread. Hier wird die add_record()-Methode gestartet. Aber an einer anderen Stelle in meinem Quelltext mache ich es dann andersrum, also übergebe der Methode die Daten. Ich erwischte mich deshalb, weil ich nicht konsequent war. Und da fragte ich mich dann "Was ist jetzt nun richtig?".

[__deets__]

Das ist exakt der Code, den ich mir in meiner Annahme vorgestellt habe. Du erzeugst einen Worker der brav alles an self bindet, und der Thread startet dann dessen add_record. Der einzige Unterschied zu meiner Annahme ist, dass du dem Qt-Muster von worker-Objekt folgst, das ein bisschen anders ist als die Art, wie man das in Python macht. Aber das ist semantisch nicht relevant fuer die Diskussion.

Und damit bleibt auch meine Antwort gleich: so macht man das nicht. Die Argumente gehoeren raus aus WorkerThread (schlechter Name, das Objekt ist ja kein Thread. Sondern ein Worker, der halt in einem Thread ausgefuehrt wird. Und ja, sowas ist wichtig, denn man soll den Code ja auch verstehen koennen).

Stattdessen sollte der Thread *einmal* zu Beginn deines Programms gestartet werden, und der Worker auf einer Queue sitzen, die dann durch eine Methode add_record aus dem Main-Thread mit einem Arbeitsauftrag bestueckt wird.

[Sophus]

@__deets__: Es ergibt sich eine weitere Frage: Den Thread, zumindest in meiner eben gezeigten Version, startet man bei Gebrauch und nicht weit im Vorfeld, mhmh? Wenn ich einen Datensatz abspeichern will, wird der Thread gestartet, alles wird abgespeichert, und danach soll der Thread wieder gelöscht werden. Daher bin ich ein wenig verwirrt, wenn du schreibst, ich soll es nur einmal starten. Vor allem, wie lässt man ein Thread solange am Leben, ehe ich es tatsächlich mal benutze? Ich komme mit deiner Formulierung nicht zurecht, wenn du schreibst "Auf einen Queue setzen".

[__deets__]

Code: Alles auswählen

class Worker:

    def __init__(self):
         self._queue = Queue()

    def add_record_invoked_from_main_thread(self, *args):
           self._queue.add(args)

     def method_invoked_by_qthread_in_started(self):
            while True:
                         work = self._queue.get() # blockiert ggf. endlos.
                         self._do_real_add_record(*work)

So sollte dein Worker aussehen, und so "sitzt" der auf einer Queue. Wahrscheinlich bekommt man das auch mit den besprochenen signal/slot Dingern hin, aber da du faul bist, bin ich es auch und suche das nicht aus der Qt-Dokumentation raus.

Und ja, der Thread lebt so lange, wie der Worker, also so lange, wie dein ganzes Programm. Aber das ist ueberhaupt kein Problem, der steht in queue.get, und verbraucht keine Resourcen.

[Sophus]

@__deets__: Da ich doch nicht soooo faul sein möchte, habe ich mir auf die Schnelle ein lauffähiges Programm geschrieben, allerdings in PyQt4. Meintest du das in Etwa so? Ich habe beim Worker eine Methode erstellt, und dort einen QTimer() platziert, damit das ganze ereignisorientiert bleibt. Denn eine endlose While-Schleife würde den Thread zu sehr beschäftigen und alles wäre blockiert. Und im Hauptthread habe ich eine add_record()-Methode erstellt, die dann beim Aufruf auf den bereits erzeugten Worker() zugreift, der ja von nun an im Haupttrhead klassenweit zur Verfügung steht.

Code: Alles auswählen

import sys

from PyQt4.QtCore import QTimer, QObject, pyqtSignal, \
     QThread, Qt

from PyQt4.QtGui import QDialog, QPushButton, \
     QApplication, QVBoxLayout,
 
class Worker(QObject):

    long_life_signal = pyqtSignal()
    finish_progress = pyqtSignal()
    
    def __init__(self, parent=None):
        QObject.__init__(self, parent)

    def long_live_worker(self):
        pass

    def start(self):

        print "Worker is started successfully"

        self.timer = QTimer()

        self.timer.setSingleShot(False)
        self.timer.setInterval(1000)
        self.timer.timeout.connect(self.long_live_worker)
        self.timer.start()

    def add_record(self, **args):
        print "args from Worker", args
        
    def stop(self):
        print 'Stop the loop'
        
        self.timer.stop()

        
class MyCustomDialog(QDialog):

    finish = pyqtSignal()

    def __init__(self, parent=None):
        QDialog.__init__(self, parent)
       
        layout = QVBoxLayout(self)
        
        self.pushButton_start = QPushButton("Start", self)
        self.pushButton_add_record = QPushButton("Add Record", self)
        self.pushButton_stopp = QPushButton("Stopp", self)
        self.pushButton_close = QPushButton("Close", self)
 
        layout.addWidget(self.pushButton_start)
        layout.addWidget(self.pushButton_add_record)
        layout.addWidget(self.pushButton_stopp)
        layout.addWidget(self.pushButton_close)
 
        self.pushButton_start.clicked.connect(self.on_start_thread)
        
        self.pushButton_add_record.clicked.connect(lambda:
                                                   self.add_record(
                                                       Name = 'Foo',
                                                       Gender = 'Unknown'))
        
        self.pushButton_stopp.clicked.connect(self.on_finish)
        
        self.pushButton_close.clicked.connect(self.close)

    def add_record(self, **kwargs):

        self.task_thread.work.add_record(Name = kwargs.get('Name'),
                                         Gender = kwargs.get('Gender'))
       
    def on_start_thread(self):
         
         self.task_thread = QThread()
         self.task_thread.work = Worker()

         self.task_thread.work.moveToThread(self.task_thread)

         self.task_thread.work.finish_progress.connect(self.task_thread.quit)

         self.finish.connect(self.task_thread.work.stop)
         
         self.task_thread.started.connect(self.task_thread.work.start)

         self.task_thread.finished.connect(self.task_thread.deleteLater)

         self.task_thread.start()

    def on_finish(self):
        
         self.finish.emit()
     
def main():
    app = QApplication(sys.argv)
    window = MyCustomDialog()
    window.resize(600, 400)
    window.show()
    sys.exit(app.exec_())
 
if __name__ == "__main__":
    main()

[__blackjack__]

@Sophus: Das macht keinen Sinn mit dem QTimer. Was soll denn da regelmässig gemacht werden? Das ist *schlechter* als die ``while True:``-Schleife in __deets__ Beispiel die nur dann tatsächlich etwas macht wenn auch ein Arbeitsauftrag rein kommt. Diese Schleife blockiert *nichts* denn wenn nichts in der Queue ist, dann verbraucht die *Null* Rechenzeit. Auch nicht alle x Millisekunden die Dein `QTimer` ja irgend etwas machen muss, auch wenn gar kein Arbeitsauftrag vorliegt. Andererseits muss jeder Arbeitsauftrag auf den `QTimer` warten, statt sofort bearbeitet zu werden.

[Sophus]

__blackjack__: Wenn ich den QTimer rausnehme, dann ist mein Worker sofort wieder "tot", mhmh? Der Timer soll einfach nur arbeiten, damit mein Worker "lebt".

UPDATE:
Ich habe eben den QTimer() entfernt, und siehe da. Der Worker lebt trotzdem. Ich bin von meinem theoretischem Gedanken ausgegangen, dass der Worker irgendwann "stirbt", wenn er nichts mehr zu erledigen hat, als ob der Gabarge Collector irgendwann aufräumt und sich sagt "Er macht nichts, brauchen wir nicht, weg damit.". So war mein Gedanke.

[__deets__]

Qthreads arbeiten (wenn wie von dir gezeigt genutzt) mit einem eigenen Eventloop. Damit müssen sie auch explizit beendet werden. Wie die Qt-eigenen Beispiele auch zeigen. https://wiki.qt.io/QThreads_general_usage

Da fordert der Worker den Thread zum quit auf. http://doc.qt.io/qt-5/qthread.html#quit

http://doc.qt.io/qt-5/qthread.html#details erklärt, das ohne überladen von run ein Event Loop gestartet wird.

Dein Worker lebt also weiter und könnte per queued Connection (!!!!!!) weiterhin Arbeitsaufträge entgegen nehmen.

[Sophus]

Schade, dass man in diesem Forum nicht auf "Danke" drücken kann. Ihr seid großartig und ich fühle mich wieder super geholfen. Besten Dank euch beiden.