Ein Script zur Berechnung des ed2k Hashwertes:
http://code.pytech.org/?page=show_paste&id=14
Wenn einer Verbesserungsvorschläge hat, immer her damit.
Und könnte es jemand unter Linux testen, plz?
MfG, Craven
ed2k hash
-
- Python-Forum Veteran
- Beiträge: 16025
- Registriert: Freitag 20. Juni 2003, 16:30
- Kontaktdaten:
Wo erwartest du denn Probleme? Ich konnte beim überfliegen zumindest keine Probleme diesbezüglich finden.Craven hat geschrieben:Und könnte es jemand unter Linux testen, plz?
Dafür aber andere: der Code ist nicht PEP8 konform, enthält keinerlei brauchbare Kommentare, dafür aber Funktionen mit nichtssagenden Namen und seltsamen Parametern (zum Beispiel würde man ``method`` eher mit None belegen und anders, einleuchtender benennen). Einen String ``string`` zu benennen ist ebenso ungeschickt, weil man dann das Modul ``string`` nicht importieren kann. Von der Einrückung mal gar nicht zu sprechen.
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
Hallo,
ich bin zwar nicht der beste Code Reviewer für Python, aber für meinen Geschmack hast du einen ziemlichen Hang zur Code Duplizierung.
Sowohl md4_hash_file und md4_hash, als auch hash_file und hash_file2 machen vom Kern her das Gleiche. Die Namen könnten klarer sein.
Dieser Code ist gleich. Den hättest du in eine eigene Funktion auslagern können. Sowas wird dann Refactoring genannt.
File open und close hättest du an einer zentralen Stelle ausführen können, z. Bsp vor der if Anweisung. Dafür müsstest du einen else Block einbauen.
Aus md4_hash_file könntest du open und close entfernen. Wenn du die Arbeit ausgeführt hast, sollte dir was auffallen.
Die if Anweisung im While Scope scheint mir unnötig.
Dann drängt sich mir noch die Frage auf, was ist den die Aufgabe der Klasse ed2k?
Na ja, hoffentlich war nun nicht zu streng.
Gruß
Markus
ich bin zwar nicht der beste Code Reviewer für Python, aber für meinen Geschmack hast du einen ziemlichen Hang zur Code Duplizierung.
Sowohl md4_hash_file und md4_hash, als auch hash_file und hash_file2 machen vom Kern her das Gleiche. Die Namen könnten klarer sein.
Code: Alles auswählen
chunksize = 9728000
if os.path.getsize(filename) <= chunksize:
return self.md4_hash_file(filename, True)
ed2k_hash = ""
fp = file(filename, 'rb')
content = True
while content != "":
content = fp.read(chunksize)
if content != "":
ed2k_hash += self.md4_hash(content)
Code: Alles auswählen
if method == True:
return hasher.hexdigest()
else:
return hasher.digest()
File open und close hättest du an einer zentralen Stelle ausführen können, z. Bsp vor der if Anweisung. Dafür müsstest du einen else Block einbauen.
Aus md4_hash_file könntest du open und close entfernen. Wenn du die Arbeit ausgeführt hast, sollte dir was auffallen.
Die if Anweisung im While Scope scheint mir unnötig.
Dann drängt sich mir noch die Frage auf, was ist den die Aufgabe der Klasse ed2k?
Na ja, hoffentlich war nun nicht zu streng.
Gruß
Markus
So, jetzt aber im richtigen Thema...
Das hier ist mit Sicherheit nicht richtig:
Zudem braucht es hier wirklich keine Klasse (auch wenn man sich manchmal merkwürdig dazu gedrängt fühlt). Es ist vollkommen ausreichend, hier nur Funktionen zu verwenden. Dadurch fallen eine Einrücktiefe und diverse `self`s weg, was die Lesbarkeit verbessert. Als Modul `ed2k` hat es bereits einen Namespace, wenn man es importiert, daher ist auch deshalb die Klasse nicht sinnvoll (in der dunklen PHP-Welt sieht das anders aus, aber hier scheint die Sonne).
Hier wird ein Wahrheitswert erwartet, der vom Vergleichsoperator geliefert wird. Allerdings geht diese Bedingung davon aus, dass `method` wahr oder falsch ist bzw. behandelt es einfach wie einen Wahrheitswert. Daher reicht dieser Code völlig:
Hier kann man Klammern zur besseren Lesbarkeit verwenden:
Würde hier zudem der Ungleich-Operator `!=` verwendet, könnte man den Teil `== 0` ähnlich dem vorigen Beispiel weglassen, da `0` zu `False` ausgewertet würde.
Mir persönlich hat es das mit Python 2.5 eingeführte `with`-Statement sehr angetan. Aus
würde dann
So ist sichergestellt, dass die Datei in jedem Fall wieder beim Verlassen des Blocks geschlossen wird.
Will man einen String zusammenfügen, sollte man die Teilstücke in eine Liste speichern und diese dann am Ende mit `''.join(string_parts)` auf einmal zusammenfügen. Da Strings keine Array-ähnlichen Typen, sondern atomar und immutable sind, müssen sie bei jedem `+=` neu angelegt werden, was sich in geringerer Performance widerspiegelt. Dazu gibt es hier im Forum auch mehrere Themen, die das näher beleuchten.
Ansonsten fällt meinem spitzfindigen Auge auf, dass deine Einrücktiefe und die Methodensignatur bei Keywords nicht den Empfehlungen von PEP 8 folgen. Auch solltest du einheitliche String-Delimiter (`'` oder `"`) verwenden und nicht abwechseln. Docstrings für die Funktionen wären natürlich auch schön.
Aber gerne doch!Craven hat geschrieben:Wenn einer Verbesserungsvorschläge hat, immer her damit.
Das hier ist mit Sicherheit nicht richtig:
Code: Alles auswählen
class ed2k():
Code: Alles auswählen
if method == True:
Code: Alles auswählen
if method:
Code: Alles auswählen
if (filesize % chunksize) == 0:
Mir persönlich hat es das mit Python 2.5 eingeführte `with`-Statement sehr angetan. Aus
Code: Alles auswählen
fp = file(filename, 'rb')
hasher = hashlib.new("md4")
hasher.update(fp.read())
fp.close()
Code: Alles auswählen
# wird in 2.6 nicht mehr erforderlich
from __future__ import with_statement
with file(filename, 'rb') as fp:
hasher = hashlib.new("md4")
hasher.update(fp.read())
Code: Alles auswählen
# ...
ed2k_hash += self.md4_hash(content)
# ...
Ansonsten fällt meinem spitzfindigen Auge auf, dass deine Einrücktiefe und die Methodensignatur bei Keywords nicht den Empfehlungen von PEP 8 folgen. Auch solltest du einheitliche String-Delimiter (`'` oder `"`) verwenden und nicht abwechseln. Docstrings für die Funktionen wären natürlich auch schön.
Naja, es ist das Aequivalent zu class ed2k:, die Klammern sind halt ueberfluessig. Falsch ist es aber nicht (bis auf die Tatsache, dass es in Python < 2.5 einen SyntaxError verursacht).Y0Gi hat geschrieben:Das hier ist mit Sicherheit nicht richtig:Code: Alles auswählen
class ed2k():
-
- Python-Forum Veteran
- Beiträge: 16025
- Registriert: Freitag 20. Juni 2003, 16:30
- Kontaktdaten:
Ich find diese Form sogar ganz nett, weil ich mir schon lange überlegt habe warum es ``def foo():`` aber ``class Foo:`` ist. Klar, sind nicht direkt so vergleichbar, aber ich finds gut, dass die jetzt mehr oder minder konsistent sind.lumax hat geschrieben:Naja, es ist das Aequivalent zu class ed2k:, die Klammern sind halt ueberfluessig. Falsch ist es aber nicht (bis auf die Tatsache, dass es in Python < 2.5 einen SyntaxError verursacht).Y0Gi hat geschrieben:Das hier ist mit Sicherheit nicht richtig:Code: Alles auswählen
class ed2k():
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
Ach stimmt, diese die-Klammern-sind-da-aber-die-Klasse-erbt-nicht-explizit-Geschichte? Gut, so gesehen passt das in der Tat. Ich habe das nur auf Anhieb für einen fälschlichen Funktionsaufruf gehalten, aber selbst das ist ja mit dieser Sichtweise nachvollziehbar.