Das deutsche Python-Forum

Hallo zusammen,

ich habe folgenden Code in einer etwas unschönen Struktur und frage mich ob man dieses Problem auch eleganter lösen kann:
Da ich den realen Code nicht posten kann, habe ich ein Beispiel konstruiert:
Der reale Code ist Teil einer Instanz-Methode, was aber für das Problem kaum eine Rolle spielt, denke ich.

Der Sinn der ganze Sache ist 'dictionary_data' in eine Gruppierungsstruktur zu mappen.

Code: Alles auswählen

class SomeClass:
    def __init__(self):
        self.dictionary_data = {"A": 1, "B": 2, "C": 3, "F": 6} # Nur als Beispiel, wird sonst aus anderer Quelle bereitgestellt.

    def do_the_mapping(self, mapping_data):
        items = {}

        for key, value in mapping_data.items():
            if isinstance(value, dict):
                item_group = {}
                for nested_key, nested_value in value.items():
                    try:
                        item_group[nested_key] = self.dictionary_data[nested_value]
                    except KeyError:
                        print(f"Missing Element {nested_key}: {nested_value}")
                    else:
                        # Sinnlose Aktion nur um zu zeigen, dass hier noch etwas mit den Werten passieren soll
                        item_group[nested_key] += 1
                        item_group[nested_key] -= 1
                items[key] = item_group
            else:
                try:
                    items[key] = self.dictionary_data[value]
                except KeyError:
                    print(f"Missing Element {key}: {value}")
                else:
                    # Sinnlose Aktion nur um zu zeigen, dass hier noch etwas mit den Werten passieren soll
                    items[key] += 1
                    items[key] -= 1
        return items


if __name__ == "__main__":
    mapping_data = {
        "Group1": {
            "AItem": "A",
            "BItem": "B"
        },
        "DItem": "D",
        "EItem": "E",
        "CItem": "C"
    }
    some_instance = SomeClass()
    items = some_instance.do_the_mapping(mapping_data)

    print(items)

Ausgabe:

Code: Alles auswählen

Missing Element DItem: D
Missing Element EItem: E
{'Group1': {'AItem': 1, 'BItem': 2}, 'CItem': 3}

Besonders die fast identischen try - except Blöcke stören mich.
Ich hatte überlegt diesen Teil in ein Methode auszulagern, bin mir aber nicht sicher ob das wirklich die beste Lösung ist.
Hat jemand Vorschläge?

Deine Datenstruktur, die sowohl aus Werten, als auch aus Wörterbüchern besteht, ist an sich schon schlecht. Warum brauchst Du das?

Dann kann man den doppelten Code in eine Funktion auslagern, und die Exception durch ein einfacheres if ersetzen:

Code: Alles auswählen

class SomeClass:
    def __init__(self):
        self.dictionary_data = {"A": 1, "B": 2, "C": 3, "F": 6} # Nur als Beispiel, wird sonst aus anderer Quelle bereitgestellt.

    def do_something_with_single_key(self, items, key, value):
        if value not in self.dictionary_data:
            print(f"Missing Element {key}: {value}")
            return
        data = self.dictionary_data[value]
        # Sinnlose Aktion nur um zu zeigen, dass hier noch etwas mit den Werten passieren soll
        data += 1
        data -= 1
        items[key] = data

    def do_the_mapping(self, mapping_data):
        items = {}
        for key, value in mapping_data.items():
            if isinstance(value, dict):
                item_group = {}
                for nested_key, nested_value in value.items():
                    self.do_something_with_single_key(item_group, nested_key, nested_value)
                items[key] = item_group
            else:
                self.do_something_with_single_key(items, key, value)
        return items

Dann muß man das Filtern der Daten von der Verarbeitung trennen:

Code: Alles auswählen

class SomeClass:
    def __init__(self):
        self.dictionary_data = {"A": 1, "B": 2, "C": 3, "F": 6} # Nur als Beispiel, wird sonst aus anderer Quelle bereitgestellt.

    def filter_mapping(self, mapping):
        for key, value in mapping.items():
            if isinstance(value, dict):
                yield key, value
            elif value in self.dictionary_data:
                yield key, value
            else:
                print(f"Missing Element {key}: {value}")

    def do_something_with_single_key(self, key, value):
        data = self.dictionary_data[value]
        # Sinnlose Aktion nur um zu zeigen, dass hier noch etwas mit den Werten passieren soll
        data += 1
        data -= 1
        return data

    def do_the_mapping(self, mapping_data):
        items = {}
        for key, value in self.filter_mapping(mapping_data):
            if isinstance(value, dict):
                items[key] = self.do_the_mapping(value)
            else:
                items[key] = self.do_something_with_single_key(key, value)
        return items

@Sirius3, Super, vielen Dank! Das sieht sehr sauber aus.

Deine Datenstruktur, die sowohl aus Werten, als auch aus Wörterbüchern besteht, ist an sich schon schlecht. Warum brauchst Du das?

Bin mir nicht ganz sicher was du meinst. Warum ist das schlecht?
Auf 'dictionary_data' habe ich allerdings auch keinen Einfluß. In der Realität ist das ein Dictionary aus längeren Zeichenketten als Schlüssel und einem custom object als Wert (Instanzen einer bestimmten Klasse).

Das deutsche Python-Forum

Mapping Algorithmus optimieren

Mapping Algorithmus optimieren

Re: Mapping Algorithmus optimieren

Re: Mapping Algorithmus optimieren