Hallo allerseits.
Da man nur vom Lesen in diesem Forum kein Python lernt, war ich gestern froh, daß ich ein konkretes Problem hatte, welches einer Lösung harrte: mein Autoradio kann zwar auf eine angeschlossene Festplatte zugreifen, allerdings gestaltet sich das Scrollen durch mehrere hundert Künstler eher anstrengend - und bevor ich das Drehrädchen irgendwann in Händen halte, sollte ein Programm her, welches mir die Ordner in einer überschaubarere Anzahl von Verzeichnissen packt, idealerweise mit gescheiter Namensgebung.
Dies ist dabei herausgekommen.
Da ich bisher kaum programmiert habe und dies letztlich mein zweites "ernsthafteres" Python-Skript darstellt, würde ich mich über ein kurzes Feedback freuen.
Dank und Gruß,
Daniel
dirsplit
-
BlackJack
@acidbath: Ich würde die Funktionen so schreiben, dass sie tatsächlich einzelne Argumente bekommen und nicht ein `args`-Argument das die Argumente für die Funktion als Attribute besitzt. Wenn man die Funktionen in einem anderen Modul verwenden möchte, müsste man sich sonst erst so ein Objekt erstellen um die Funktionen aufrufen zu können.
`os.walk()` nur für die erste Ebene zu verwenden, sieht für mich ein bisschen nach einem Hack aus. Auf jeden Fall wird dort die eingebaute `dir()`-Funktion verdeckt obwohl der Wert noch nicht einmal im weiteren Programmverlauf verwendet wird. Hier sieht man oft den Namen `_` für Werte die man binden muss, aber nicht verwendet. Etwas weniger oft auch als Präfix für einen sprechenden Bezeichner.
Das `files` in `get_source_dirs_and_files()` an `None` gebunden werden kann, ist IMHO unschön. Man könnte eine leere Liste nehmen — dann bleibt die Struktur des Ergebnisses wenigstens immer gleich. Man könnte das Flag hier aber auch einfach gar nicht berücksichtigen ohne das es im weiteren Programmverlauf dadurch zu einem Problem kommen dürfte.
Es wird sehr oft, aber nicht immer (!), `abs()` verwendet um aus ungültigen Benutzereingaben gültige zu machen. Auf eine IMHO nicht sinnvolle Weise. Wenn jemand eine negative Verzeichnisanzahl oder Länge angibt, dann ist das ein Fehler. Den sollte man nicht durchgehen lassen, sondern dem Benutzer um die Ohren hauen. Selbst wenn man das mit `abs()` korrigieren möchte, sollte man das an *einer* Stelle im Quelltext tun, und nicht an (fast) jeder Stelle wo man den Wert verwenden möchte.
Beim Erstellen der Verzeichnisstruktur sehe ich das Problem das `args.length` zu klein gewählt werden kann wenn wirklich sehr viele Verzeichnisse mit dem gleichen Präfix beginnen. Mal vom pathologischen Fall, dass hier jemand 0 vorgibt ganz zu schweigen.
In der ersten Schleife in `move_dirs()` wird `entries` anscheinend gar nicht verwendet. Das würde ich wie oben schon erwähnt entsprechend kennzeichnen.
Für die Anzeige von Dateinamen für den Benutzer würde ich nicht '"%s"' sondern '%r' verwenden. Das ist die `repr()`-Form, da werden automatisch Anführungszeichen drum herum gesetzt *und* alle ”exotischen” Bytewerte werden escaped, damit der Benutzer sehen kann woraus die Namen tatsächlich bestehen.
Bei den Kommandozeilenoptionen sind einige lange Optionen fälschlicherweise als kurze angegeben. Statt `-src` müsste es `--src` heissen. Ein '-' bedeutet auch nur ein Buchstabe, denn sonst kann es zu Mehrdeutigkeiten kommen. Denn üblicherweise kann man die kurzen Optionen zusammenfassen, dass heisst wenn es '-s', '-r', und '-c' kann man das als '-s -r -c' angeben, aber auch als '-src' (oder '-scr' oder …).
IMHO sind benötigte Optionen ein Widerspruch. Wenn eine Option angegeben werden *muss*, dann ist es keine Option mehr, denn der Name kommt ja von optional. Das Programm braucht ein Zielverzeichnis um arbeiten zu können. Das ist nicht optional.
Beim Hilftstext zur '-sep'-Option fehlen die Leerzeichen.
`os.walk()` nur für die erste Ebene zu verwenden, sieht für mich ein bisschen nach einem Hack aus. Auf jeden Fall wird dort die eingebaute `dir()`-Funktion verdeckt obwohl der Wert noch nicht einmal im weiteren Programmverlauf verwendet wird. Hier sieht man oft den Namen `_` für Werte die man binden muss, aber nicht verwendet. Etwas weniger oft auch als Präfix für einen sprechenden Bezeichner.
Das `files` in `get_source_dirs_and_files()` an `None` gebunden werden kann, ist IMHO unschön. Man könnte eine leere Liste nehmen — dann bleibt die Struktur des Ergebnisses wenigstens immer gleich. Man könnte das Flag hier aber auch einfach gar nicht berücksichtigen ohne das es im weiteren Programmverlauf dadurch zu einem Problem kommen dürfte.
Es wird sehr oft, aber nicht immer (!), `abs()` verwendet um aus ungültigen Benutzereingaben gültige zu machen. Auf eine IMHO nicht sinnvolle Weise. Wenn jemand eine negative Verzeichnisanzahl oder Länge angibt, dann ist das ein Fehler. Den sollte man nicht durchgehen lassen, sondern dem Benutzer um die Ohren hauen. Selbst wenn man das mit `abs()` korrigieren möchte, sollte man das an *einer* Stelle im Quelltext tun, und nicht an (fast) jeder Stelle wo man den Wert verwenden möchte.
Beim Erstellen der Verzeichnisstruktur sehe ich das Problem das `args.length` zu klein gewählt werden kann wenn wirklich sehr viele Verzeichnisse mit dem gleichen Präfix beginnen. Mal vom pathologischen Fall, dass hier jemand 0 vorgibt ganz zu schweigen.
In der ersten Schleife in `move_dirs()` wird `entries` anscheinend gar nicht verwendet. Das würde ich wie oben schon erwähnt entsprechend kennzeichnen.
Für die Anzeige von Dateinamen für den Benutzer würde ich nicht '"%s"' sondern '%r' verwenden. Das ist die `repr()`-Form, da werden automatisch Anführungszeichen drum herum gesetzt *und* alle ”exotischen” Bytewerte werden escaped, damit der Benutzer sehen kann woraus die Namen tatsächlich bestehen.
Bei den Kommandozeilenoptionen sind einige lange Optionen fälschlicherweise als kurze angegeben. Statt `-src` müsste es `--src` heissen. Ein '-' bedeutet auch nur ein Buchstabe, denn sonst kann es zu Mehrdeutigkeiten kommen. Denn üblicherweise kann man die kurzen Optionen zusammenfassen, dass heisst wenn es '-s', '-r', und '-c' kann man das als '-s -r -c' angeben, aber auch als '-src' (oder '-scr' oder …).
IMHO sind benötigte Optionen ein Widerspruch. Wenn eine Option angegeben werden *muss*, dann ist es keine Option mehr, denn der Name kommt ja von optional. Das Programm braucht ein Zielverzeichnis um arbeiten zu können. Das ist nicht optional.
Beim Hilftstext zur '-sep'-Option fehlen die Leerzeichen.
Moin Blackjack.
Vielen Dank für Deine Mühe!
Da werde ich mir mein Skript wohl noch einmal zur Brust nehmen müssen. Dein Feedback zeigt, daß es nochmal einen riesen Unterschied macht, ob man ein Programm "mal eben schnell" für sich selbst zusammenbastelt oder als "echtes Softwareprojekt" (ich weiß, in diesem Zusammenhang etwas hochtrabend) ggf. sogar veröffentlichen wollen würde.
Gruß,
Daniel
Vielen Dank für Deine Mühe!
Da werde ich mir mein Skript wohl noch einmal zur Brust nehmen müssen. Dein Feedback zeigt, daß es nochmal einen riesen Unterschied macht, ob man ein Programm "mal eben schnell" für sich selbst zusammenbastelt oder als "echtes Softwareprojekt" (ich weiß, in diesem Zusammenhang etwas hochtrabend) ggf. sogar veröffentlichen wollen würde.
Gruß,
Daniel
Hallo nochmal.
Leider ist meine Zeit momentan eher knapp bemessen, aber trotzdem möchte ich noch einmal genauer auf Deine Nachricht eingehen - immerhin hast Du ja auch einige Deiner Zeit investiert.
Der neue Quelltext befindet sich hier.
Die Nutzung des Unterstrichs für nicht benötigte Werte war mir neu. Danke.
Hat da jemand auf die Schnelle eine Idee? Spontan würde ich ja sagen, einfach eine Zählvariable hinten anhängen, etwa: [01/07], [02/07], ...
So, wenn jetzt noch jemand mit ähnlich vielen Korrekturen kommt, kollabiere ich.
Gruß,
Daniel
Leider ist meine Zeit momentan eher knapp bemessen, aber trotzdem möchte ich noch einmal genauer auf Deine Nachricht eingehen - immerhin hast Du ja auch einige Deiner Zeit investiert.
Der neue Quelltext befindet sich hier.
Stimmt, das ergibt Sinn und ich hab's (hoffentlich korrekt) umgesetzt.BlackJack hat geschrieben:@acidbath: Ich würde die Funktionen so schreiben, dass sie tatsächlich einzelne Argumente bekommen und nicht ein `args`-Argument das die Argumente für die Funktion als Attribute besitzt. Wenn man die Funktionen in einem anderen Modul verwenden möchte, müsste man sich sonst erst so ein Objekt erstellen um die Funktionen aufrufen zu können.
Stimmt, das os.walk() mutet komisch an, ehrlich gesagt bin ich damit den Weg des geringsten Widerstands gegangen, weil ich ja genau an den zwei Dingen interessiert bin: Dateien und Verzeichnisse. Mit os.listdir() hätte ich diese Unterteilung händisch vornehmen müssen.BlackJack hat geschrieben:`os.walk()` nur für die erste Ebene zu verwenden, sieht für mich ein bisschen nach einem Hack aus. Auf jeden Fall wird dort die eingebaute `dir()`-Funktion verdeckt obwohl der Wert noch nicht einmal im weiteren Programmverlauf verwendet wird. Hier sieht man oft den Namen `_` für Werte die man binden muss, aber nicht verwendet. Etwas weniger oft auch als Präfix für einen sprechenden Bezeichner.
Die Nutzung des Unterstrichs für nicht benötigte Werte war mir neu. Danke.
Hut ab, daß Du sowas so schnell siehst. Ist entfernt.BlackJack hat geschrieben:Das `files` in `get_source_dirs_and_files()` an `None` gebunden werden kann, ist IMHO unschön. Man könnte eine leere Liste nehmen — dann bleibt die Struktur des Ergebnisses wenigstens immer gleich. Man könnte das Flag hier aber auch einfach gar nicht berücksichtigen ohne das es im weiteren Programmverlauf dadurch zu einem Problem kommen dürfte.
Ich habe eine Funktion verify_arguments hinzugefügt, die die Kontrolle hoffentlich korrekt durchführt. Die ständige Wiederholung war in der Tat dämlich und wieder einmal der Faulheit geschuldet. Zunächst ging ich nur von mir aus, dachte erst später daran, daß andere Nutzer potentiell auch negative Werte eingeben könnten --- naja, und dachte mir 'ach komm, knallste eben mal ein abs vor und gut is'.BlackJack hat geschrieben:Es wird sehr oft, aber nicht immer (!), `abs()` verwendet um aus ungültigen Benutzereingaben gültige zu machen. Auf eine IMHO nicht sinnvolle Weise. Wenn jemand eine negative Verzeichnisanzahl oder Länge angibt, dann ist das ein Fehler. Den sollte man nicht durchgehen lassen, sondern dem Benutzer um die Ohren hauen. Selbst wenn man das mit `abs()` korrigieren möchte, sollte man das an *einer* Stelle im Quelltext tun, und nicht an (fast) jeder Stelle wo man den Wert verwenden möchte.
Stimmt, bei den Dopplungen muß ich mir noch etwas einfallen lassen.BlackJack hat geschrieben:Beim Erstellen der Verzeichnisstruktur sehe ich das Problem das `args.length` zu klein gewählt werden kann wenn wirklich sehr viele Verzeichnisse mit dem gleichen Präfix beginnen. Mal vom pathologischen Fall, dass hier jemand 0 vorgibt ganz zu schweigen.
Hat da jemand auf die Schnelle eine Idee? Spontan würde ich ja sagen, einfach eine Zählvariable hinten anhängen, etwa: [01/07], [02/07], ...
Danke.BlackJack hat geschrieben:In der ersten Schleife in `move_dirs()` wird `entries` anscheinend gar nicht verwendet. Das würde ich wie oben schon erwähnt entsprechend kennzeichnen.
Und wieder was gelernt.BlackJack hat geschrieben:Für die Anzeige von Dateinamen für den Benutzer würde ich nicht '"%s"' sondern '%r' verwenden. Das ist die `repr()`-Form, da werden automatisch Anführungszeichen drum herum gesetzt *und* alle ”exotischen” Bytewerte werden escaped, damit der Benutzer sehen kann woraus die Namen tatsächlich bestehen.
Danke. Ich bin nicht so sehr auf der Kommandozeile beheimatet, deshalb war mir diese Konvention neu.BlackJack hat geschrieben:Bei den Kommandozeilenoptionen sind einige lange Optionen fälschlicherweise als kurze angegeben. Statt `-src` müsste es `--src` heissen. Ein '-' bedeutet auch nur ein Buchstabe, denn sonst kann es zu Mehrdeutigkeiten kommen. Denn üblicherweise kann man die kurzen Optionen zusammenfassen, dass heisst wenn es '-s', '-r', und '-c' kann man das als '-s -r -c' angeben, aber auch als '-src' (oder '-scr' oder …).
Stimmt - Sprachgefühl, wo steckst Du?BlackJack hat geschrieben:IMHO sind benötigte Optionen ein Widerspruch. Wenn eine Option angegeben werden *muss*, dann ist es keine Option mehr, denn der Name kommt ja von optional. Das Programm braucht ein Zielverzeichnis um arbeiten zu können. Das ist nicht optional.
Korrigiert.BlackJack hat geschrieben:Beim Hilftstext zur '-sep'-Option fehlen die Leerzeichen.
So, wenn jetzt noch jemand mit ähnlich vielen Korrekturen kommt, kollabiere ich.
Gruß,
Daniel