Das deutsche Python-Forum

Hallo,

ich habe ein Programm geschrieben, dass zufällig (Modul random) eine Person aus einer Liste mit Personen auswählen kann oder zufällige Gruppen bildet.
Beim Ausführen des Programms habe ich aber leider festgestellt, dass es bei der 2 Wiederholung zu einem TypeError kommt,
welchen ich allerdings nicht verstehen oder nachvollziehen kann. Kann mir vllt. jemand erläutern, was der Fehler ist.
Ich habe die Idee, das es damit zusammen hängt, dass aus der list_of_people beim ersten Durchlauf Sachen entfernt werden, wodurch es beim zweiten mal nicht läuft.
Allerdings weiß ich nicht wieso das sein könnte, da ich die Liste ja nur als Argument übergebe und nicht Veränderungen vornehmen lasse.
Außerdem bin ich mir unsicher, welche Version ich in Zeile 47-57 brauche.

Code: Alles auswählen

# This Program should make it possible to chose a random Person from a list or to make random groups

# Imports
from random import*

# Fill this List with the names of the people
list_of_people = ["Camilla", "Sixta", "Inez", "Jen", "Stefanie", "Darleen", "Martha", "Peg", "Johanna", "Sun",
                  "Tobi", "Fleta", "Arden", "Adrien", "Megan", "Sibyl", "Dario", "Roselia", "Tyson", "Anna"]


def make_groups(size_of_groups, list_of_people_arg):
    """Takes the group size and the list of people and returns a list with the groups."""
    # list_of_people_arg = list_of_people_arg
    local_list = []  # List to append at list_of_groups
    list_of_groups = []  # Will store the different groups
    number_of_people = len(list_of_people_arg)
    if not number_of_people < size_of_groups:  # Checking for a valid group size
        number_of_remaining_people = number_of_people  # Assign loop variable
        while number_of_remaining_people >= size_of_groups:  # Checking if enough people are in the list
            number_of_remaining_people -= size_of_groups
            for i in range(size_of_groups):  # Loop for adding people to the lists
                local_list.append(choice(list_of_people_arg))
                list_of_people_arg.remove(local_list[-1])
            list_of_groups.append(local_list)
            local_list = []
        return list_of_groups
    else:
        print(f"There are not enough people to make groups. Here are all people: {list_of_people_arg}")


while True:
    input1 = input("""Enter groups and comma separated the size of them with or person 
                      if you want the Program to make random groups or chose a random person!
                      (groups,(number)/person)""").lower()
    split_input1 = input1.split(",")
    if 2 == len(split_input1):  # Checks if groups was chosen
        try:
            split_input1[1] = int(split_input1[1])

        except ValueError:
            print("Need valid input!")
            continue

    if split_input1[0] == "groups":
        list_of_groups = make_groups(split_input1[1], list_of_people)  # --) Function
        """if not len(list_of_people) > int(split_input1[1]):  #
            print("Here are the groups")
            for i in range(len(list_of_groups)):  # Should print the groups, but here occurs the error if input is an even number
                print(list_of_groups[i])
            if len(list_of_people) != 0:
                print(list_of_people)"""
        print("Here are the groups")
        for i in range(len(list_of_groups)):  # Should print the groups, but here occurs the error if input is an even number
            print(list_of_groups[i])
        if len(list_of_people) != 0:
            print(list_of_people)

    elif split_input1[0] == "person":  # Simple code for choosing a person
        random_person = choice(list_of_people)
        print(random_person)
    else:
        print("Need valid input!")

Console:

Enter groups and comma separated the size of them with or person
if you want the Program to make random groups or chose a random person!
(groups,(number)/person)groups,4
Here are the groups
['Megan', 'Darleen', 'Inez', 'Fleta']
['Tobi', 'Adrien', 'Sixta', 'Peg']
['Sun', 'Martha', 'Johanna', 'Jen']
['Sibyl', 'Camilla', 'Tyson', 'Dario']
['Anna', 'Stefanie', 'Arden', 'Roselia']
Enter groups and comma separated the size of them with or person
if you want the Program to make random groups or chose a random person!
(groups,(number)/person)groups,4

Traceback (most recent call last):
There are not enough people to make groups. Here are all people: []
File "C:/Users/Private/PycharmProjects/Python_for_Teachers/Chose_People_from_List.py", line 54, in <module>
Here are the groups
for i in range(len(list_of_groups)): # Should print the groups, but here occurs the error if input is an even number
TypeError: object of type 'NoneType' has no len()

Process finished with exit code 1

Danke im Voraus

PS: Wie nutze ich bei PyCharm die Debug Funktion?

@Redstonerayy: Der Fehler kommt dadurch zustande das die `make_groups()`-Funktion implizit auch `None` zurückgeben kann und `None` hat halt keine Länge. Die Funktion geht ja in den letzten ``else``-Zweig wie man an der Ausgabe sehen kann, und da steht nirgends ein ``return`` das etwas zurück gibt mit der Code der die Funktion aufruft etwas anfangen kann, denn der will auf jeden Fall etwas das eine Länge hat.

Code: Alles auswählen

In [5]: len(None)                                                               
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-96b07938784c> in <module>
----> 1 len(None)

TypeError: object of type 'NoneType' has no len()

Die Funktion sollte an der Stelle am besten keine Ausgabe per `print()` machen sondern eine Ausnahme auslösen, und die entsprechende Behandlung und Ausgabe für den Benutzer dann an der Stelle passieren wo die Funktion aufgerufen wird.

Weitere Anmerkungen zum Code: Der erste Kommentar wäre eine gute Grundlage für den Docstring des Moduls.

Kommentare sollten einen Mehrwert bieten. Über den Importen zu ``# Imports`` zu kommentieren bringt dem Leser keine nützliche Information. Faustregel: Kommentare sollten nicht beschreiben *was* passiert, denn das steht da bereits als Code, sondern warum der Code das *so* macht. Sofern das nicht offensichtlich ist. Bevor man die Bedeutung von Namen kommentiert, sollte man schauen ob man nicht einen besseren Namen findet, so dass man den nicht erklären muss.

Sternchen-Importe machen Programme unübersichtlicher, weil man als Leser nicht immer alle Namen kennt die damit definiert werden. Es sind ja auch nicht nur die Namen die für das Modul dokumentiert sind und die in dem Modul definiert werden, sondern auch Namen die das Modul selbst irgendwo anders her importiert. Ich sehe auf Anhieb nicht welche Funktionen aus `random` verwendet werden, dazu muss ich das gesamte Programm absuchen und *wissen* wonach ich suche.

Konkrete Grunddatentypen haben in Namen nichts zu suchen. Das ändert sich öfter mal während der Programmentwicklung, sei es weil man einen passenderen Typ aus der Standardbibliothek oder externen Paketen verwendet, oder weil man sich irgendann selbst einen spezifischeren Typ für etwas schreibt was am Anfang mal eine Liste, ein Wörterbuch, oder ähnliches war. Da muss man dann entweder das ganze Programm absuchen und alle betroffenen Namen ändern, oder man hat irreführende Namen im Programm. `local_list` ist ein besonders nichtssagender Name. Beide Informationen die in dem Namen stehen sind ganz offensichtlich am Code ablesbar, egal wie der Name lauten würde. Das ist ein lokaler Name — wird als erstes in der Funktion definiert — und es wird bei der Definition eine Liste an den Namen gebunden. Was da fehlt wäre die Information was denn in dieser Liste gespeichert — was für eine Bedeutung sie im Code hat.

Konstanten schreibt man in Python komplett in Grossbuchstaben.

Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst.

Namen sollte man nicht nummerieren. Besonders unsinnig ist es einfach irgendwo eine 1 an einen Namen zu hängen wenn es noch nicht einmal den Namen mit einer 2 gibt. Wenn man Namen durchnummeriert will man sich entweder bessere Namen ausdenken, oder gar keine Einzelnamen vergeben sondern eine Datenstruktur verwenden. Oft eine Liste.

`_arg` an Argumentnamen anzuhängen macht auch keinen Sinn. Das sieht der Leser doch welche Namen Argumente sind. Funktionen sollten nicht so lang werden dass man das nicht mehr leicht ablesen kann.

`make_groups()` könnte ein bisschen besser strukturiert sein. Die Bedingung die zu der Fehlermeldung führt, und die eigentlich zu einer Ausnahme führen sollte, kann man gleich ganz an den Anfang ziehen, denn man braucht davor gar erst *irgendwas* zu machen, wenn die Aufgabe der Funktion mit den gegebenen Argumenten gar nicht erfüllt werden kann. Damit spart man sich dann auch gleich eine Einrückungsebene für den Rest der Funktion.

`local_list` wird zu früh definiert. Das sollte *in* der ``while``-Schleife am *Anfang* stehen. Dann braucht diese gleiche Zuweisung einer leeren Liste auch nicht *zweimal* im Code stehen haben. Das der Name schlecht ist, schrob ich ja weiter oben schon. Da werden die Namen einer Gruppe gesammelt, also wäre `group` ein sinnvoller Name dafür.

Zwei Namen `number_of_people` und `number_of_remaining_people` für die gleiche Information zu verwenden ist ein Name zu viel. Eigentlich braucht man dafür gar keinen Namen, da Du ja Namen aus der Liste entfernst, also `number_of_remaining_people` immer den gleichen Wert hat wie die Länge der Liste. Dann kann man auch die Länge der Liste abfragen statt die ausserhalb der Liste noch mal parallel mit zu zählen.

Wenn man aus syntaktischen Gründen einen Namen braucht, den Namen aber nicht verwendet, gibt es die Konvention `_` als Namen zu benutzen.

Die `make_groups()`-Funktion könnte dann so aussehen:

Code: Alles auswählen

def make_groups(group_size, names):
    """
    Take the group size and the names and return a list with the groups.
    """
    if len(names) < group_size:
        raise ValueError(
            f"There are not enough names to make groups."
            f" Here are all names: {names}"
        )

    groups = []
    while len(names) >= group_size:
        group = []
        for _ in range(group_size):
            group.append(random.choice(names))
            names.remove(group[-1])
        groups.append(group)
    
    return groups

Was daran äusserst unschön ist, ist das die Liste mit den Namen verändert wird. Funktion sollten nicht solche Nebeneffekte haben, und wenn, dann sollte man die *deutlich* dokumentieren, denn damit rechnet der Leser nicht. Man könnte hier beispielsweise am Anfang der Funktion eine Kopie der Liste anlegen, was zusätzlich den Vorteil hätte, das die Funktion nicht nur mit einer Namens*liste* klar käme, sondern mit jedem beliebigen endlichen iterierbaren Objekt. Und da hätte wir dann auch schon ein Beispiel wo `list_of_names` oder `names_list` plötzlich falsch und irreführend als Argumentname wäre wenn man den Code durch eine einzige Zeile so ändert das es nicht zwingend eine Liste sein muss.

Wenn man die Namen in einer lokalen Liste sammelt, kann man das ganze auch noch viel effizienter schreiben als immer einzelne Namen aus der Liste auszuwählen und aus der einen Liste entfernen um sie in eine andere Liste zu stecken. Man kann einfach die Namen zufällig mischen und dann die Ergebnislisten per „slicing“ erstellen.

Code: Alles auswählen

def make_groups(group_size, names):
    """
    Take the group size and the names and return a list with the groups.
    """
    names = list(names)
    group_count = len(names) // group_size
    
    if group_count == 0:
        raise ValueError(
            f"There are not enough names to make groups."
            f" Here are all names: {names}"
        )

    random.shuffle(names)
    return [
        names[i * group_size : i * group_size + group_size]
        for i in range(group_count)
    ]

Man könnte sich auch überlegen ob man den Sonderfall mit keiner Gruppe überhaupt so ”besonders” behandeln muss, und in dem Fall nicht einfach eine leere Liste zurück gibt, was ja nicht einmal extra Code benötigen würde.

Auch in der Hauptfunktion könnte der Code besser strukturiert sein. Das erste ``if`` gehört eigentlich *in* den Fall das man schon weiss, dass der Benutzer "groups" eingegeben hat, denn nur dann benötigt man die Umwandlund des zweiten Elements in eine ganze Zahl. Die sollte man dann aber nicht mehr in die Liste zurück speichern. In Listen sollte man Objekte speichern die alle den gleichen (Duck-)Typ haben, nicht Zeichenketten und Zahlen mischen.

Der Kommentar bei dem ``if`` ist auch falsch, denn da wird nicht geprüft ob der Benutzer "groups" eingegeben hat, sondern ob er genau ein Komma in der Eingabe hatte.

``continue`` sollte man vermeiden. Das ist ein unbedingter Sprung an den Schleifenanfang den man nicht an der Einrückung des Quellcodes erkennen kann. Das kann sehr schnell sehr unübersichtlich werden.

Das hantieren mit ”magischen” Indexwerten ist nicht schön. Statt also immer mit 0 und 1 auf die gesplittete Eingabe zuzugreifen, sollte man die Teile an Namen binden. Also mindestens mal den Wert an Index 0, denn das ist ja immer der Name des Kommandos das ausgeführt werden soll.

Literale Zeichenketten sind nicht dazu da um Code auszukommentieren.

``for i in range(len(sequence)):`` ist in Python ein „anti-pattern“. Man kann direkt über die Elemente von Sequenztypen wie Listen iterieren, ohne den Umweg über einen Laufindex.

Zwischenstand:

Code: Alles auswählen

#!/usr/bin/env python3
"""
This Program allows to chose a random person from a list or to make random
groups.
"""
import random

NAMES = [
    "Adrien",
    "Anna",
    "Arden",
    "Camilla",
    "Dario",
    "Darleen",
    "Fleta",
    "Inez",
    "Jen",
    "Johanna",
    "Martha",
    "Megan",
    "Peg",
    "Roselia",
    "Sibyl",
    "Sixta",
    "Stefanie",
    "Sun",
    "Tobi",
    "Tyson",
]


def make_groups(group_size, names):
    """
    Take the group size and the names and return a list with the groups.
    """
    names = list(names)
    random.shuffle(names)
    return [
        names[i * group_size : i * group_size + group_size]
        for i in range(len(names) // group_size)
    ]


def main():
    while True:
        command, *arguments = (
            input(
                "Enter groups and comma separated the size of them with or person\n"
                "if you want the Program to make random groups or chose a random person!\n"
                "(groups,(number)/person/quit): "
            )
            .lower()
            .split(",")
        )

        if command == "groups":
            try:
                group_size = int(arguments[0])
            except (IndexError, ValueError):
                print("Need a number as argument!")
            else:
                groups = make_groups(group_size, NAMES)
                if groups:
                    print("Here are the groups")
                    for group in groups:
                        print(group)
                else:
                    print(
                        f"There where not enough names for given"
                        f" group size {group_size}."
                    )

        elif command == "person":
            print(random.choice(NAMES))

        elif command == "quit":
            break

        else:
            print(f"Unknown command {command!r}")


if __name__ == "__main__":
    main()

Hey Blackjack,

danke für die Hilfe. Mein Programm läuft jetzt.
Allerdings war es auch noch so, dass beim 2. Durchlauf die Liste mit Namen verändert war, z. B. leer. Liegt das daran, dass die Funktion die Liste beeinflusst?

Dank, Redstonerayy

@Redstonerayy: In der Originalfunktion hast Du die übergebene Liste verändert in dem da Elemente rausgelöscht wurden. Das hatte ich ja im letzten Beitrag schon gesagt, dass das unschön ist. Wenn man so etwas macht, sollte man das *deutlich* dokumentieren, weil das den Leser überrascht und zu schwer zu findenden Problemen führen kann wenn man Datenstrukturen verändert die sonst wo ihren Ursprung haben können.

Das deutsche Python-Forum

Unverstänlicher TypeError

Unverstänlicher TypeError

Re: Unverstänlicher TypeError

Re: Unverstänlicher TypeError

Re: Unverstänlicher TypeError