Einstieg: ToDo schrittweise ausbauen (Support needed)

[kasmo]

Hi zusammen,

ich habe mich immer wieder mal mit Python beschäftigt. Dann wieder lange Pause nichts, dann doch wieder angefangen usw. Dann die 100days of Python angefangen, aber nicht sehr weit gekommen.

Jetzt habe ich mir vorgenommen eine ToDo Liste von Grund auf selbst zu machen und nach und nach mit "Googlen" und z.B. hier, weitere Funktionen zu ergänzen und zu lernen.

Mein größtes Problem habe ich gerade mit Dictionary und class.

Könntet ihr mir sagen, ob das so einigermaßen korrekt ist? Mir kommt es komisch vor, aus einem Dictionary eine Klasseninstanz zu ziehen. (Beispiel mit einer Klasse kam aus FB von einem User).

Code: Alles auswählen

todo_list = {}


class Task:
  def __init__(self, name, day, description=""):
    self.name = name.capitalize()
    self.day = day
    self.description = description

  def __str__(self):
    return f"Name: {self.name}, Day: {self.day}, Description: {self.description}"

  def add_task():
    name = input("What is the name of your task? ").capitalize()
    day = input("Which day for your task? ")
    description = input("Place optional an description for your task! ")
    todo_list[name] = Task(name, day, description)

  def del_task():
    #Ausgabe der Aufgaben
    for key, value in todo_list.items():
      print(f"Name: {key}")
      print("Description:", value.description)
      print("Day:", value.day)
      print("############\n\n")

    #Wähle die Aufgabe die gelöscht werden soll anhand des Namens.
    while True:
      del_choice = input("Please choose the Name of Task to delete. ").capitalize()
      if del_choice in todo_list:
        todo_list.pop(del_choice, None)
        break
      else:
        ask = input("Name is not in the Tasklist, please r for repeat or a for abort! ").lower()
        if ask == "a":
          break

[__blackjack__]

@kasmo: Eingerückt wird per Konvention vier Leerzeichen pro Ebene.

Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. `todo_list` hat da nichts zu suchen. Das `_list` ist auch ein bisshen irreführend, weil es gar keine Liste ist.

`add_task()` und `del_task()` sind ja wohl keine Methoden von `Task` denn die machen ja gar nichts mit dem Task auf dem sie definiert sind. Sie bekommen nicht mal den Task als Argument, was falsch ist für eine Methode. Das sind einfach Methoden. Und die müssen `todo_list` als Argument bekommen, denn sonst können sie da gar nicht drauf zugreifen, weil das sollte ja nicht global sein.

Man sollte nicht `key` und `value` als Namen verwenden wenn man da *bessere* Namen verwenden kann. Das sind `name` und `task`. Und eigentlich braucht man den Schlüssel nicht, denn der Wert ist ja auch als Attribut auf dem `Task`-Objekt vorhanden.

`pop()` ist die Methode wenn man tatsächlich etwas mit dem Wert machen möchte. Das Default-Argument macht nur Sinn wenn es überhaupt sein kann, dass es den Schlüssel nicht gibt. Das ist an der Stelle aber sicher.

`ask` ist kein sinnvoller Name für eine Antwort vom Benutzer.

Bei der Abbrechen/Wiederholen Frage wird der Benutzer eigentlich angelogen. Es ist entweder 'a' oder irgend etwas anderes, und nicht 'a' oder 'r'.

Ungetestet:

Code: Alles auswählen

#!/usr/bin/env python3


class Task:
    def __init__(self, name, day, description=""):
        self.name = name.capitalize()
        self.day = day
        self.description = description

    def __str__(self):
        return (
            f"Name: {self.name},"
            f" Day: {self.day},"
            f" Description: {self.description}"
        )


def add_task(name_to_task):
    name = input("What is the name of your task? ").capitalize()
    name_to_task[name] = Task(
        name,
        input("Which day for your task? "),
        input("Enter an optional description for your task! "),
    )


def del_task(name_to_task):
    for task in name_to_task.values():
        print(f"Name: {task.name}")
        print("Description:", task.description)
        print("Day:", task.day)
        print("############\n\n")

    while True:
        name = input("Please choose the name of task to delete. ").capitalize()
        if name in name_to_task:
            del name_to_task[name]
            break

        while True:
            answer = input(
                "Name is not in the tasklist, please r for repeat"
                " or a for abort! "
            ).lower()
            if answer == "a":
                return
            elif answer == "r":
                break


def main():
    name_to_task = {}
    ...


if __name__ == "__main__":
    main()

[Sirius3]

Eingerückt wird immer mit 4 Leerzeichen pro Ebene; bei 2en sieht alles so gedrängt aus, dass es schwierig zu lesen ist.
Es darf keine globalen Variablen geben. Warum hat ein Task eine Funktion `add_task`? Dass das `self` fehlt, zeigt deutlich, dass das keine Methode der Klasse sein sollte.
Eine Klasse ToDoList könnte so eine Methode haben, dann erledigt sich das auch gleich mit der globalen Variablen.
todo_list ist ein Wörterbuch, hat aber list im Namen. Das ist verwirrend. Ich würde ja sagen, eine Todo-Liste ist auch tatsächlich eine Liste und kein Wörterbuch.

[kasmo]

Kurz vorab, wieso ist mit Firefox meine IP im Spamfilter? Mit Chrome und Mobile Hotspot geht es jetzt zum Glück?!

@kasmo: Eingerückt wird per Konvention vier Leerzeichen pro Ebene.

Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. `todo_list` hat da nichts zu suchen. Das `_list` ist auch ein bisshen irreführend, weil es gar keine Liste ist.

`add_task()` und `del_task()` sind ja wohl keine Methoden von `Task` denn die machen ja gar nichts mit dem Task auf dem sie definiert sind. Sie bekommen nicht mal den Task als Argument, was falsch ist für eine Methode. Das sind einfach Methoden. Und die müssen `todo_list` als Argument bekommen, denn sonst können sie da gar nicht drauf zugreifen, weil das sollte ja nicht global sein.

Man sollte nicht `key` und `value` als Namen verwenden wenn man da *bessere* Namen verwenden kann. Das sind `name` und `task`. Und eigentlich braucht man den Schlüssel nicht, denn der Wert ist ja auch als Attribut auf dem `Task`-Objekt vorhanden.

`pop()` ist die Methode wenn man tatsächlich etwas mit dem Wert machen möchte. Das Default-Argument macht nur Sinn wenn es überhaupt sein kann, dass es den Schlüssel nicht gibt. Das ist an der Stelle aber sicher.

`ask` ist kein sinnvoller Name für eine Antwort vom Benutzer.

Bei der Abbrechen/Wiederholen Frage wird der Benutzer eigentlich angelogen. Es ist entweder 'a' oder irgend etwas anderes, und nicht 'a' oder 'r'.

.....

Das hat mir mega geholfen dankeschön. Auch an den anderen Beitrag vielen lieben Dank.

Ich habe jetzt bestimmt eine Stunde gebraucht um das alles zum laufen zu kriegen nach Deinen Tipps und dem Code Beispiel. Hat echt lange gedauert, bis ich es einigermaßen gecheckt habe mit dem Wert name_to_task übergeben.

Das hier ist mein erweitertes Ergebnis. Ist das jetzt besser oder hab ich noch wo unnütze Aufrufe? Ist das korrekt, das wirklich jede Funktion name_to_task übergeben kriegen muss? Speziell beim MENU finde ich irgendwie, das es extrem oft auftaucht, sieht komisch aus

Code: Alles auswählen

class Task:
    def __init__(self, name, day, description=""):
        self.name = name.capitalize()
        self.day = day
        self.description = description

    def __str__(self):
        return f"Name: {self.name}, Day: {self.day}, Description: {self.description}"

def show_task_list(name_to_task):
    for task in name_to_task.values():
        print("Name: ", task.name)
        print("Description: ", task.description)
        print("Day: ", task.day)
        print("############\n\n")
    


def add_task(name_to_task):
    name = input("What is the name of your task? ").capitalize()
    name_to_task[name] = Task(name, input("Which day for your task? "), input("Place an optional Description!" ))
    print("Task added successfully!")
    
    print(name_to_task[name])
    menu(name_to_task)

def del_task(name_to_task):
  #Ausgabe der Aufgaben
  show_task_list(name_to_task)

  #Wähle die Aufgabe die gelöscht werden soll anhand des Namens.
  while True:
      name = input("Please choose the Name of Task to delete. ").capitalize()
      if name in name_to_task:
          del name_to_task[name]
          print("Delete success")
          break

      while True:
          answer = input("Your Choice is not in the Tasklist. Please r for repeat "
                          "or a for abort: ")
          if answer == "r":
              break
          elif answer == "a":
              menu(name_to_task)
          else:
              print("Please enter r or a")

def update_task(name_to_task):
    show_task_list(name_to_task)
    update_task = input("Please choose the Name of Task to update. ").capitalize()
    if update_task in name_to_task:
        print("What do you want to update? ")
        print("1. Name")
        print("2. Day")
        print("3. Description")
        update_choice = int(input("What do you want to update? "))
        if update_choice == 1:
            new_name = input("What is the new name? ").capitalize()
            name_to_task[new_name] = name_to_task[update_task]
            name_to_task[new_name].name = new_name
            del name_to_task[update_task]
            print("Update success")
        elif update_choice == 2:
            new_day = input("What is the new day? ")
            name_to_task[update_task].day = new_day
            print("Update success")
        elif update_choice == 3:
            new_description = input("What is the new description? ")
            name_to_task[update_task].description = new_description
            print("Update success")
    menu(name_to_task)

def menu(name_to_task): 
    menu = {
        1: "Add Task",
        2: "Delete Task",
        3: "Show Task List",
        4: "Update Task",
        5: "Exit"
    }
    for x, y in menu.items():
        print(x, ":", y)

    choice = int(input("What do you want to do? "))
    if choice == 1:
        add_task(name_to_task)
    elif choice == 2:
        del_task(name_to_task)
    elif choice == 3:
        show_task_list(name_to_task)
    elif choice == 4:
        update_task(name_to_task)
    elif choice == 5:
        print("Bye!")

def main():
    name_to_task = {}
    menu(name_to_task)

[Sirius3]

Eingerückt wird immer mit 4 Leerzeichen pro Ebene, in del_task sind es nur 2.
Funktionen erledigen eine Aufgabe und kehren dann zum Aufrufer zurück und rufen nicht wieder die Anfangsfunktion (hier menu) auf, denn das führt zu einer unübersichtlichen Aufrufstruktur. Aus `del_task` kommt man nie wieder raus, egal wie oft man "Bye" sagt.
Statt dessen packt man das menu selbst wieder in eine Schleife.
Der Aufruf von Task in `add_task` ist unübersichtlich, weil alles in einer Zeile steht; __blackjack__ hat das mit Zeilenumbrüchen lesbarer gemacht, ich würde aber, wie Du ursprünglich, zu ein paar mehr Variablen tendieren.
x und y sind schlechte Variablennamen für den Menu-Index und Menu-Eintrag.

Wenn eine Funktion name_to_task braucht, muß sie es als Argument bekommen.
Alternative wäre eine TodoList-Klasse.

Code: Alles auswählen

class Task:
    def __init__(self, name, day, description=""):
        self.name = name.capitalize()
        self.day = day
        self.description = description

    def __str__(self):
        return f"Name: {self.name}, Day: {self.day}, Description: {self.description}"

def show_task_list(name_to_task):
    for task in name_to_task.values():
        print("Name: ", task.name)
        print("Description: ", task.description)
        print("Day: ", task.day)
        print("############\n\n")


def add_task(name_to_task):
    name = input("What is the name of your task? ")
    day = input("Which day for your task? ")
    description = input("Place an optional Description!" )
    task = Task(name, day, description)
    name_to_task[task.name] = task
    print("Task added successfully!")
    print(task)


def del_task(name_to_task):
    show_task_list(name_to_task)

    #Wähle die Aufgabe die gelöscht werden soll anhand des Namens.
    while True:
        name = input("Please choose the Name of Task to delete. ").capitalize()
        if name in name_to_task:
            del name_to_task[name]
            print("Delete success")
            break

        while True:
            answer = input(
                "Your Choice is not in the Tasklist. "
                "Please r for repeat or a for abort: "
            )
            if answer == "r":
                break
            elif answer == "a":
                return
            else:
                print("Please enter r or a")


def update_task(name_to_task):
    show_task_list(name_to_task)
    update_task = input("Please choose the Name of Task to update. ").capitalize()
    if update_task in name_to_task:
        print("What do you want to update? ")
        print("1. Name")
        print("2. Day")
        print("3. Description")
        update_choice = int(input("What do you want to update? "))
        if update_choice == 1:
            new_name = input("What is the new name? ").capitalize()
            name_to_task[new_name] = name_to_task[update_task]
            name_to_task[new_name].name = new_name
            del name_to_task[update_task]
            print("Update success")
        elif update_choice == 2:
            new_day = input("What is the new day? ")
            name_to_task[update_task].day = new_day
            print("Update success")
        elif update_choice == 3:
            new_description = input("What is the new description? ")
            name_to_task[update_task].description = new_description
            print("Update success")


def menu(name_to_task): 
    menu = {
        1: "Add Task",
        2: "Delete Task",
        3: "Show Task List",
        4: "Update Task",
        5: "Exit"
    }
    for index, entry in menu.items():
        print(f"{index}: {entry}")

    choice = int(input("What do you want to do? "))
    if choice == 1:
        add_task(name_to_task)
    elif choice == 2:
        del_task(name_to_task)
    elif choice == 3:
        show_task_list(name_to_task)
    elif choice == 4:
        update_task(name_to_task)
    elif choice == 5:
        print("Bye!")

def main():
    name_to_task = {}
    while True:
        menu(name_to_task)

if __name__ == "__main__":
    main()

[kasmo]

Danke, hab ich direkt umgesetzt

Aber ich komme trotzdem noch nicht aus der Menu Schleife (5. Exit) raus. Wenn ich 5 für Exit drücke, kommt trotzdem das Menü. Habe auch schon return False bei 5 ausprobiert.

[Kebap]

Wie wäre es, wenn du nicht nur "return" hinzufügst, sondern den zurück gegebenen Wert dann auch prüfst und zur Entscheidung nutzt?

Code: Alles auswählen

...
    elif choice == 5:
        print("Bye!")
        return -1

def main():
    name_to_task = {}
    while True:
        result = menu(name_to_task)
        if result == -1: 
            break

if __name__ == "__main__":
    main()

[kasmo]

Wie wäre es, wenn du nicht nur "return" hinzufügst, sondern den zurück gegebenen Wert dann auch prüfst und zur Entscheidung nutzt?

Code: Alles auswählen

...
    elif choice == 5:
        print("Bye!")
        return -1

def main():
    name_to_task = {}
    while True:
        result = menu(name_to_task)
        if result == -1: 
            break

if __name__ == "__main__":
    main()

Danke, das war es
Ja da hast du wohl recht. Vor lauter Code (für mich schonmal sehr viel für den Anfang) sehe ich gar nichts mehr und muss jedes mal alles nochmal komplett von vorne durchlesen um den gesamten Ablauf überhaupt zu verstehen xD