Auslagern von gleichen Methoden in Klassen

[Kniffte]

Hallo Zusammen,

Ich möchte gern in folgendem Code die Import und Export Methoden auslagern, so dass diese nicht in jeder einzelnen Klasse wiederholt werden.
Habe dabei an Dekoratoren gedacht oder sollte ich das in einer separate Klasse machen und dann mit Vererbung arbeiten?
Finde leider nicht den richtigen Ansatz und benötige eure Hilfe.

Gruß Seb

Code: Alles auswählen

# def import(sender):
#     pass

# def export(recepient):
#     pass


class Blade:

    def __init__(self):
        self.length = 10
        self.height = 50
        
        self.data_to_export = {'length': self.length,
                               'height': self.height}

    # @ Decorator ... ?!
    def import_data(self, sender_dict=None):
        if isinstance(sender_dict, dict):
            for key, value in self.data_to_export.items():
                if value != sender_dict[key]:
                    value = sender_dict[key]

    def export_data(self, recipient_dict=None):
        pass


class Edge:

    def __init__(self):
        self.x_coord = 10
        self.y_coord = 50
        self.type = 'le'
        
        self.data_to_export = {'x_coord': self.x_coord,
                               'y_coord': self.y_coord,
                               'type': self.type}

    # @ Decorator ... ?!
    def import_data(self, sender_dict=None):
        if isinstance(sender_dict, dict):
            for key, value in self.data_to_export.items():
                if value != sender_dict[key]:
                    value = sender_dict[key]

    def export_data(self, recipient_dict=None):
        pass

[BlackJack]

@Kniffte: Wenn es einen gemeinsamen Obertyp von `Edge` und `Blade` gibt, dann wäre wahrscheinlich eine (abstrakte) Basisklasse eine Idee.

[Sirius3]

@Kniffte: ja, macht man per Vererbung, oder per Dekorator auf die Klasse. `data_to_export` ist ein Designfehler. Die Werte in diesem Wörterbuch ändern sich nicht, sondern sind Kopien der Attribute; das sollte entweder ein Parameter der Attributnamen für den Dekorator sein, oder eine Klassenkonstante. import_data macht gar nichts, außer ein paar lokale Variablenzuweisungen, die keinen Einfluß auf das Programm haben.

[Kniffte]

... import_data macht gar nichts, außer ein paar lokale Variablenzuweisungen, die keinen Einfluß auf das Programm haben.

@Sirius:
Das hab ich soweit rausgefunden, aber wohl nicht so recht verstanden, wie ich es abändern muss in Bezug auf deinen Hinweis?

...das sollte entweder ein Parameter der Attributnamen für den Dekorator sein, oder eine Klassenkonstante...

Code: Alles auswählen

class Blade:

    def __init__(self):
        self.length = 10
        self.height = 50

        self.data_to_export = {'length': self.length,
                               'height': self.height}

    # @ Decorator ... ?!
    def import_data(self, sender_dict):
        if isinstance(sender_dict, dict):
            for key, value in self.data_to_export.items():
                if value != sender_dict[key]:
                    value = sender_dict[key]


if __name__ == '__main__':
    a = Blade()
    print(a.length)
    print(a.height)

    s_dict = {'length': 5,
              'height': 99}

    a.import_data(s_dict)
    print(a.length)
    print(a.height)

Bzw. muss ich jedes Attribut einzeln zuweisen? (siehe import_data_2)

Code: Alles auswählen

class Blade:

    def __init__(self):
        self.length = 10
        self.height = 50

        self.data_to_export = {'length': self.length,
                               'height': self.height}

    # @ Decorator ... ?!
    def import_data(self, sender_dict):
        if isinstance(sender_dict, dict):
            for key, value in self.data_to_export.items():
                if value != sender_dict[key]:
                    value = sender_dict[key]

    def import_data_2(self, sender_dict):
        if isinstance(sender_dict, dict):
            try:
                self.length = sender_dict['length']
                self.height = sender_dict['height']
            except KeyError:
                pass


if __name__ == '__main__':
    a = Blade()
    print(a.length)
    print(a.height)

    s_dict = {'length': 5,
              'height': 99}

    a.import_data(s_dict)
    print(a.length)
    print(a.height)

    a.import_data_2(s_dict)
    print(a.length)
    print(a.height)

jetzt macht zumindest der import was...

[Kniffte]

Spricht was gegen diese Varianten der 'import_data' methode?
Hat eine eher Nachteile oder ist ein 'code smell'?

Code: Alles auswählen

class Blade:

    def __init__(self):
        self.length = 10
        self.height = 50

    def import_data(self, sender_dict):
        if isinstance(sender_dict, dict):
            for key, value in sender_dict.items():
                if value != self.__dict__[key]:
                    self.__dict__[key] = value

    def import_data_1(self, sender_dict):
        if isinstance(sender_dict, dict):
            for key, value in sender_dict.items():
                if value != self.__getattribute__(key):
                    self.__setattr__(key, value)

[BlackJack]

@Kniffte: Ich persönlich würde vermeiden direkt auf die ”magischen” Attribute zuzugreifen und `getattr()`/`setattr()` verwenden.

[Sirius3]

@Kniffte: so

Code: Alles auswählen

class ImportExportMixin:
    def import_data(self, data):
        for key in self.PUBLIC_ATTRIBUTES:
            setattr(self, key, data[key])

    def export_data(self):
        result = {}
        for key in self.PUBLIC_ATTRIBUTES:
            result[key] = getattr(self, key)


class Blade(ImportExportMixin):
    PUBLIC_ATTRIBUTES = ['length', 'height']

    def __init__(self):
        self.length = 10
        self.height = 50

[Kniffte]

@Kniffte: Ich persönlich würde vermeiden direkt auf die ”magischen” Attribute zuzugreifen und `getattr()`/`setattr()` verwenden.

Danke... hab´s auch so abgeändert.


@Sirius:
Danke für das Bsp. Nun hab ich´s vestanden...macht´s sehr übersichtlich und verständlich

Ich habe zwischenzeitlich noch ne eigene Variante geschrieben:

Code: Alles auswählen

class ImportExport:
    def import_data(self, sender_dict):
        if isinstance(sender_dict, dict):
            for key, value in sender_dict.items():
                if value != getattr(self, key):
                    setattr(self, key, value)

    def export_data(self, recipient_dict=None):
        if isinstance(recipient_dict, dict):
            for key, value in recipient_dict.items():
                if value != getattr(self, key):
                    recipient_dict[key] = getattr(self, key)
        return self.__dict__
        # return {key:value for key, value in self.__dict__.items()}


class Blade(ImportExport):

    def __init__(self):
        self.length = 10
        self.height = 50
    

class Edge(ImportExport):

    def __init__(self):
        self.x_coord = 10
        self.y_coord = 50
        self.type = 'le'

Spricht was gegen diese Variante?
Beim 'return self.__dict__' bin ich mir nicht sicher, ob das 'sauber' ist oder es auch anders geht, wenn ich alle attribute exportieren will als Wörterbuch?

[Sirius3]

@Kniffte: was willst Du eigentlich machen? Die Abfragen mit isinstance sind sehr schlecht. Nicht nur, dass damit alle anderen wörterbuchähnlichen Contrainertypen ausgeschlossen werden, wenn man etwas anderes angibt, wird einfach nichts gemacht. Das sollte nie passieren. Ohne isinstance wird einfach ein IndexError oder KeyError geworfen, und dann weiß man woran man ist.

export_data soll Daten exportieren? Dann ist es ungewöhnlich, dass man Daten übergeben muß. Die Prüfung, ob der Wert gleich ist, oder sich geändert hat, ist unnötig.

Sei so explizit wie möglich; gib die Attribute an, die Du exportieren willst. So kannst Du interne Attribute intern belassen. Normalerweise will man nicht direkt __dict__ übergeben, wenn man was exportiert. Denn jede Änderung am Rückgabewert ändert auch gleich das ursprüngliche Objekt.

[Kniffte]

@Sirius:
Danke für deine Hinweise.
Leider fehlt mir die Erfahrung und dadurch oft der Gesamtüberblick, um eine Programmieraufgabe auf Anhieb effektiv zu lösen.
Was mir anfangs sinnvoll erscheint, stellt sich nach weiterer Recherche und Unterstützung von euch hier im Forum, als umständlich oder teils unsinnig heraus. Aber grundsätzlich ist es, denke ich, nicht vekehrt zuerst selbst nen Lösung zu finden und danach weiter zu optimieren.
Aktuell schreibe ich mein erstes Programm mittels OOP und das ist schon ne gewisse Herausforderung was Struktur, Übersicht und Funktion angeht. Vorallem wenn man bisher eher in kleinem Umfang prozedural und funktional in anderen Sprachen programmiert hat.
Ich denke ich bin auf nem guten Weg, was das Programm angeht. Ich hab es zwar schon 3 mal wieder komplett umgebaut, weil ich mit Struktur und Übersicht bei zunehmender Codegröße nicht zufrieden war, aber dabei hab ich auch viel lernen können und aus Fehlern lernt man meist am besten.

@Sirius & @BlackJack:
An der Stelle möcht ich euch beiden mal Danke sagen, da ihr beiden sehr aktiv im Forum Hilfestellung gebt und eure Erläuterungen, Tipps und Codebeispiele mich immer voran gebracht haben. (Zumindest in den Themen, die ich vorrangig durchforste)

@Sirius:
Also ich hab die Klassenkonstanten laut deinem Vorschlag übernommen und statt 'isinstance' Ausnahme abgearbeitet.
Den direkten Export von __dict__ erstze ich durch ne Dictionary Comprehension, die auch mit den Klassenkonstanten arbeitet.