Das deutsche Python-Forum

Hallo!
Ich hab mal wieder etwas "gestrickt". Man könnte vermutlich main() noch abspecken und für das holen der Files eine eigene Funktion schreiben. Abgesehen davon, auch noch einiges viel besser machen.

Sollte main() grundsätzlich ausschließlich andre Funktionen aufrufen und möglichst schlank gehalten werden?

Ist mein try / except Block bzw. mein while mit 2x break (je nach eintretender Bedingung) in Ordnung?

Bitte auch um allgemeine Diskussion

-- die es sicher geben wird...

Code: Alles auswählen

from glob import glob
from os import rename, chdir, getcwd
def get_dir():
    while True:
        try:
            working_dir = input ("Arbeitsverzeichnis (Return = Startverzeichnis): ")
            if working_dir == "":
                working_dir = getcwd()
                print ("Bleibe in", getcwd())
                break
            else:
                chdir(working_dir)
                print ("Wechsle Verzeichnis nach: ", working_dir)
                break
            
        except FileNotFoundError:
            print("Verzeichnis nicht gefunden!")
    return working_dir
    

def main():
    index = 0
    directory = get_dir()
    extension = input ("Welche Erweiterung: (Bsp: jpg)")
    files = "*." + extension
    filelist = glob(files)
    count_files = len(filelist)
    print ("Gefundene ", extension, " - Dateien: ", count_files)
    new_name = input("Wie soll der Namenteil NEU lauten: ")
    for elemente in filelist:
        index +=1
        file_name = new_name + "_" + str(index) + "." + extension
        rename(elemente, file_name) 
        print (elemente, "->", file_name)
    
    
if __name__ == "__main__":
    main()

LG
Daniel

Hallo.

Ich gehe einfach mal der Reihe nach durch:

- Du solltest an deiner Namensgebung arbeiten. "dir" kannst du auch als "directory" ausschreiben, die Abkürzungen bringen hier nicht viel. Auch sind Namen wie "index" oder "directory" sehr nichtssagend.

- Eingabe/Ausgabe und Verarbeitung sollten immer getrennt werden. Du solltest also eine Funktion haben um das Arbeitsverzeichnis vom Benutzer zu erfragen und eine Funktion um da rein zu wechseln.

- Wenn du auf einen leeren String testen möchtest, dann reicht ein ``if not working_dir``. Wenn du testen möchtes, ob etwas im String steckt, dann einfach ``if working_dir``.

- Die ``breaks`` solltest du zu einem ``break`` zusammenfassen. Wenn man etwas im if-Block und im else-Block macht, dann sollte man es aus den Blöcken rausziehen. Damit vermeidest du doppelten Code.

- Das Verzeichnis zu wechseln ist keine gute Idee, damit veränderst du den globalen Zustand des Programms. Das sollte man so wenig wie möglich machen, da dies immer eine mögliche Fehlerquelle ist und zu schwer auffindbaren Fehlern kommt. Stell dir vor, eine andere Funktion tut das selbe und du mischt die Funktionen irgendwo. Das führt in jedem Fall zu einem Chaos. Gib lieber den Pfad zurück -- das machst du ja auch jetzt schon -- und setze ihn bei Bedarf zusammen.

- Deine index-Variable ist unnötig. Wenn du zu einem Element einen Index brauchst, dann verwende dazu die enumerate-Funktion:

Code: Alles auswählen

for index, elemente in enumerate(filelist)

- Entscheide dich für eine Sprache: Englisch oder Deutsch, aber mische nicht beides.

- Strings solltest du mittels String-Formatting zusammensetzen und nicht mit +.

- In "files" stecken keine Dateien, sondern ein MUSTER für DATEINAMEN.

- "filelist" ist auch wieder ein schlechter Name. "filenames" ist passender. Als kleine Orientierung: wenn du Datentypen in Namen verwendest, dann solltest du dir über die Namensgebung noch einmal Gedanken machen. Das ist häufig ein Hinweis auf einen Fehler oder etwas, was man übersehen hat.

- "count_files" ist auch wieder ein schlechter Namen. Hier wird nichts gezählt, es handelt sich um eine ANZAHL. Wirf mal einen Blick in ein Wörterbuch.

- "elemente" ist wieder ein ganz schlechter Name. Du solltest es danach benennen, WAS für Elemente darin gespeichert sind. In diesem Fall Dateinamen. Außerdem sind es nicht mehrere Elemente, sondern genau ein Dateiname. "filename" ist also der passende Bezeichner.

- Ja, es steckt zu viel in deiner main-Funktion. Hier kann man doch prima in verschiedene Funktionen aufteilen. Noch einmal, damit es deutlich wird: Trenne Eingabe/Ausgabe und Verarbeitung. Dann ist kein Code übersichtlicher und wiederverwendbar.

@mcdaniels: `chdir()` sollte man nicht verwenden. Das ändert einen prozessweiten Zustand und es skaliert nicht. Wenn man das in grösseren Programmen an mehreren Stellen versucht, kommt es früher oder später dazu, dass Funktionen auf effektiven Pfaden ausgeführt werden, mit denen man so nicht gerechnet hat. Die Funktion gibt das Verzeichnis zurück, aber damit wird beim Aufrufer gar nichts gemacht. Statt des `chdir()` sollte man den Rückgabewert auch tatsächlich verwenden um den Pfad zusammenzusetzen mit dem das Programm dann irgend etwas tut.

Wenn etwas am Anfang oder am Ende *aller* möglichen ``if``/``elif``/``else``-Zweige steht, kann man es auch *einmal* vor beziehungsweise nach diesem Konstrukt ausführen. Das betrifft in der `get_dir()` das ``break``. Anstelle des einen ``break`` nach der Abfrage, könnte man auch gleich das ``return`` dort ausführen lassen. Und wenn man den Pfad zurück gibt und nicht `chdir()` verwendet, dann braucht man die Sonderbehandlung der leeren Zeichenkette auch nicht.

`print()` ist eine Funktion wie jede andere, also sollte man da kein Leerzeichen setzen was man bei anderen Funktionsaufrufen auch nicht setzt. (Okay, bei `input()` machst Du das anscheinend auch — warum?)

In der `main()` wird `directory` gar nicht verwendet. Sollte es aber wie gesagt, statt `chdir()`.

`files` ist ein unpassender Name, da würde man Dateiobjekte (*mehrzahl*!) erwarten, aber keine Zeichenkette mit einem Glob-Muster. Ähnliches gilt für `filelist`. Das sind Datei*namen* und keine Dateien. Ausserdem sollte man konkrete Datentypen in Namen vermeiden, da man die öfter man im Laufe der Entwicklung durch andere Datentypen austauscht, und dann sind entweder die Namen irreführend oder man muss sie überall ändert. Beides begünstigt Fehler.

Auch `elemente` ist ein schlechter Name. Zum einen stimmt hier der Plural wieder nicht, denn es sind nicht mehrere Elemente der Liste die an diesen Namen gebunden werden, sondern immer nur *einer*. Und das jeweilige Element kann man sicher besser benennen als so generisch. Die Schleife geht über eine Liste mit Dateinamen, also ist ein Element ein Dateiname.

`index` wird viel zu weit vor der Schleife initialisiert. Wenn man den Quelltext liest, und an die Stelle kommt, wo es dann endlich benutzt wird, muss man sich das bis dorthin gemerkt haben, oder sucht erst einmal wieder rückwärts durch den Quelltext wo eigentlich die erste Zuweisung war. Zumal dieses manuelle hochzählen kürzer und übersichtlicher mit der `enumerate()`-Funktion gemacht werden kann.

Werte mit ``+`` und `str()` zu Zeichenketten zusammenzusetzen ist eher BASIC denn Python. In Python gibt es die `format()`-Methode auf Zeichenketten dafür.

Die Ausgabe was da umbenannt wird, sollte *vor* dem `rename()`-Aufruf gemacht werden. Sollte bei dem etwas schief laufen, zum Beispiel ein Rechteproblem, dann sieht der Benutzer wenigstens bei welcher Datei das Problem aufgetaucht ist.

Ich komme dann auf etwas in dieser Art (ungetestet):

Code: Alles auswählen

import os
from glob import glob


def get_dir():
    while True:
        result = input('Arbeitsverzeichnis (Return = Startverzeichnis): ')
        if os.path.isdir(result):
            return result
        print('Verzeichnis nicht gefunden!')
    

def main():
    directory = get_dir()
    extension = input('Welche Erweiterung: (Bsp: jpg) ')
    pattern = os.path.join(directory, '*.' + extension)
    filenames = glob(pattern)
    print('Gefundene ', extension, '- Dateien: ', len(filenames))
    new_name = input('Wie soll der Namenteil NEU lauten: ')
    for index, filename in enumerate(filenames, 1):
        new_filename = os.path.join(
            directory, '{0}_{1}.{3}'.format(new_name, index, extension)
        )
        print(filename, '->', new_filename)
        os.rename(filename, new_filename) 
    
    
if __name__ == '__main__':
    main()

Hallo Leute,
vielen Dank für die Rückmeldungen. Das muss ich jetzt erstmal verinnerlichen bzw. das Ganze mal in kleine Häppchen zerteilen um es in den Kopf zu kriegen

@blackjack:

`print()` ist eine Funktion wie jede andere, also sollte man da kein Leerzeichen setzen was man bei anderen Funktionsaufrufen auch nicht setzt. (Okay, bei `input()` machst Du das anscheinend auch — warum?)

Hat sich offenbar eingeschlichen, danke für den Hinweis!

LG
Daniel

Das deutsche Python-Forum

Aufteilung in Funktionen bzw. main() Aufruf

Aufteilung in Funktionen bzw. main() Aufruf

Re: Aufteilung in Funktionen bzw. main() Aufruf

Re: Aufteilung in Funktionen bzw. main() Aufruf

Re: Aufteilung in Funktionen bzw. main() Aufruf