Programmcode verbessern/vereinfachen?

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
Unplayable
User
Beiträge: 51
Registriert: Mittwoch 24. Februar 2016, 22:09

Hallo Zusammen,

das folgende Programm war eine Hausaufgabe. Ich sollte überprüfen, ob eine Liste nur Nullen und Einsen enthält. Außerdem dürfen in der Liste maximal 3 Einsen hintereinander vorkommen.

Code: Alles auswählen

def gueltige_liste(liste,zahl1,zahl2):


    if liste == []:                                             # gleicht ab, ob die Liste einen Inhalt hat
        return False

    elif len(liste) > 3:                                        # überprüft, ob mehr als 3 Einsen in der Liste hintereinander vorkommen
        if liste[1] and liste[2] and liste[3] == liste[0]:
            return False

    
    if liste[0] > zahl1 or liste[0] < zahl2:                    # wenn ein Element der Liste größer als 1 oder kleiner als 0 ist, wird False zurückgegeben
        return False

    else:
        
        if len(liste) == 1:                                     # wenn nur noch ein Element in der Liste verbleibt, ist die Liste gültig
            return True

        else:
            return gueltige_liste(liste[1:],zahl1,zahl2) 

Jetzt wollte ich mal wissen, ob euch da irgenwelche Sachen auffallen, die man vielleicht vereinfachen oder verbessern könnte.
Im Voraus vielen Dank :)
__deets__
User
Beiträge: 14529
Registriert: Mittwoch 14. Oktober 2015, 14:29

Ein paar Anmerkungen:

- zahl1 und zahl2 sind ganz schlecht benannt. Du solltest die nach ihrer tatsaechlichen Aufgabe benennen.
- es ist komisch, dass du mal parameter nimmst, mal hard-kodierte Abfragen. Entweder muss zahl2 0 sein, oder es darf variabel sein. Aber dann greifen deine Bedingungen nicht mehr.
- der Test auf die 4-Elementige Teilliste aus Einsen ist sehr, sehr verwirrend, und meines Erachtens auch falsch. Jedenfalls ist deiner Beschreibung nach

Code: Alles auswählen

gueltige_liste([0, 1, 1, 0, 1], 1, 0)
eigentlich eine gueltige Liste, aber deine Funktion liefert False zurueck.

Wenn ich das mit dem viel einfacheren

Code: Alles auswählen

    if liste[:4] == [1, 1, 1, 1]:
            return False
ersetze kommt fuer mich das richtige Ergebnis raus.
Unplayable
User
Beiträge: 51
Registriert: Mittwoch 24. Februar 2016, 22:09

Okay danke schon mal
Benutzeravatar
noisefloor
User
Beiträge: 3856
Registriert: Mittwoch 17. Oktober 2007, 21:40
Wohnort: WW
Kontaktdaten:

Hallo,

es bietet sich an, hier mit einem Set zu arbeiten. Liste in ein Set überführen und dann prüfen, ob das Set gleich `set([0, 1])` ist.

Für die 2. Frage würde ich entweder `collections.deque` zum Einsatz bringen oder (vielleicht einfacher und einsteigerverständlicher), über die Liste iterieren, einen Slice mit 4 Elementen zurückgeben lassen, diesen in ein Set überführen und prüfen, ob das gleich `set([1])` ist.

Gruß, noisefloor
Benutzeravatar
noisefloor
User
Beiträge: 3856
Registriert: Mittwoch 17. Oktober 2007, 21:40
Wohnort: WW
Kontaktdaten:

Hallo,

Nachtrag: `groupyby` aus den `itertools` wäre wahrscheinlich noch eleganter.

Gruß, noisefloor
Sirius3
User
Beiträge: 17741
Registriert: Sonntag 21. Oktober 2012, 17:20

@Unplayable: Funktionsnamen sollten eine Tätigkeit ausdrücken, "gueltige_liste" kann entweder heißen, dass eine Liste auf Gültigkeit geprüft wird, dass eine gültige Liste erzeugt wird, dass eine Liste nach bestimmten Kriterien in eine gültige Liste umgewandelt wird, etc. Ein besserer Name wäre `check_validity` was genau das ausdrückt, was gemacht werden soll. Der Code an sich ist schlecht lesbar, das liegt zum einen an den vielen Leerzeilen, die den Code zerreißen, so dass die Einheit "Funktion" optisch nicht heraussticht. Zum zweiten die Kommentar am Ende der Zeilen, die in meinem Editor alle abgeschnitten sind. Kommentare sollten auch beschreiben, wie etwas gemacht wird und nicht was, weil das aus dem Code schon herauskommt. Außer natürlich, dass der Kommentar etwas anderes beschreibt, als der Code macht, wie die Prüfung auf mehr als 3 Einsen, die so nicht funktioniert. In Zeile 12 sind zwar Code und Kommentar konsistent, aber machen nicht das, was die Aufgabenstellung verlangt.
Warum ist die leere Liste ungültig?

Die Aufgabenstellung würde ich auch nicht rekursiv lösen. Python hat ein sehr striktes Rekursions-Limit. Auch ist das Problem iterativ genauso einfach zu lösen.

@noisefloor: so? `all(v in [1,0] and sum(b) <= 3 for v, b in groupby(iterable))`
Antworten