Das deutsche Python-Forum

Airmp3.com ist eine Meta-MP3-Suchmaschine, welche vier Musiksuchmaschinen abfragt. Dieses Tool erstellt aus den Suchergebnissen von airmp3.com Playlisten im m3u-Format.

Edit (Leonidas): Code ausgelagert.

Warum nutzt du nicht einen anständigen HTML-Parser? Dann würde der Code sicher weniger fürchterlich aussehen.

@snakeseven: Das ist ziemlich, äh Barock. Gross, umständlich, schlecht entworfen, teilweise ziemlich unsinnig.

Nur mal so schnell durchgeschaut:

Aus der `__init__()` wird nicht ersichtlich welche Attribute die Klasse im Laufe ihres Lebens haben wird.

`loadpagesources()`: Warum wird `src` übergeben aber nicht verwendet? Was ist bitte `bol`? Wozu soll `online` gut sein? Warum gibt die Funktion (denn eine Methode scheint das vom Inhalt her nicht zu sein) einen "Fehlercode" zurück? Dafür wurden Ausnahmen erfunden.

Genauso in `single_line_parser()`, wobei der Boolean-Wert da auch noch redundant ist, denn anscheinend ist das dritte Element immer genau dann `False` wenn das erste die leere Zeichenkette ist, und `True` wenn im ersten Element keine leere Zeichenkette steht. Aber auch hier wäre eine Ausnahme angebrachter.

Methodennamen sollten beschreiben was die Methode tut. Das kann ich bei `airmp3()` nicht erkennen. Ausserdem ist die Methode definitiv zu lang und zu komplex.

`checkifnumbers()` wird nirgends benutzt, aber falls es benutzt würde, was wäre der Vorteil gegenüber der `isdigit()`-Methode auf Zeichenketten? Selbst wenn man diese Methode nicht kennt *und* sonst eher in "low level"-Sprachen programmiert, ist die Funktion ziemlich gruselig. Das würde man selbst in C oder BASIC nicht so umständlich lösen.

Diese ganze HTML-Parserei und das hin- und herkodieren ist ein schlechter Witz.

``try``/``except`` ohne konkrete Ausnahme und dann auch noch mit ``pass`` ist Böse™.

Leonidas hat geschrieben:Warum nutzt du nicht einen anständigen HTML-Parser? Dann würde der Code sicher weniger fürchterlich aussehen.

Hi Leonidas,
Das Programm ist ein Auszug aus einem Metasuchprgramm, welches über 20-100 parallele Threads Suchanfragen an verschiedene MP3-Suchmaschinen stellt und auswertet. Die Verwendung von Beautiful Soup fand ich da tempomäßig nicht überzeugend, weswegen ich das Ganze "zu Fuss" gemacht habe. Es ist ja auch gar nicht nötig, eine HTML Seite erst in alle ihre Bestandteile zu zerlegen, um dann gezielt bestimmte Teile (z.B. Tabellen) auszuwerten. Dann kann ich das doch gleich machen ? Ich weiss ja, wo ich im Sourcecode nachsehen muss.

Gruss, Seven

snakeseven hat geschrieben:Die Verwendung von Beautiful Soup fand ich da tempomäßig nicht überzeugend

Natürlich nicht, aber BeautifulSoup sehe ich eher als interessanten Hack denn als anständigen Parser. Wenn es um Geschwindigkeit geht, ist lxml das mittel der Wahl. Eigentlich ist lxml fast immer das Mittel der Wahl....

BlackJack hat geschrieben:@snakeseven: Das ist ziemlich, äh Barock. Gross, umständlich, schlecht entworfen, teilweise ziemlich unsinnig.

Nur mal so schnell durchgeschaut:

Aus der `__init__()` wird nicht ersichtlich welche Attribute die Klasse im Laufe ihres Lebens haben wird.

Weil in einigen Methoden der Klasse weitere Attribute hinzugefügt werden?
Ich weiss, sollte man lassen. Wirklich schlimm finde ich es zwar nicht, sollte mich aber bemühen, es in Zukunft anders zu machen.

`loadpagesources()`: Warum wird `src` übergeben aber nicht verwendet? Was ist bitte `bol`? Wozu soll `online` gut sein? Warum gibt die Funktion (denn eine Methode scheint das vom Inhalt her nicht zu sein) einen "Fehlercode" zurück? Dafür wurden Ausnahmen erfunden.

'src' wird an 'source1', bzw. 'source2' übergeben, 'bol' ist ein boolscher Wert, der zeigt, ob der Sourcecode der Seite geladen werden konnte, oder nicht. Mit try/except wäre es natürlich auch gegangen. So aber kann ich mir detailiert anzeigen lassen, was da schief gelaufen ist.

Genauso in `single_line_parser()`, wobei der Boolean-Wert da auch noch redundant ist, denn anscheinend ist das dritte Element immer genau dann `False` wenn das erste die leere Zeichenkette ist, und `True` wenn im ersten Element keine leere Zeichenkette steht. Aber auch hier wäre eine Ausnahme angebrachter.

Ja, klar, mit try/except kann man ganz schlank und übersichtlichen Code erstellen. Aber mir ist auch hier wichtig, jeden Schritt des Miniparsers überprüfen und ausgeben zu können. Hier arbeite ich nur mit einer Suchmaschine. Wenn es aber 10 sind, oder mehr, dann gibt es immer wieder Situationen, wo unsauberer HTML-Code zu putzigen Ergebnissen führt. Das will ich mir dann einfach in einem der 'else'-Zeilen ausgeben lassen können.

Methodennamen sollten beschreiben was die Methode tut. Das kann ich bei `airmp3()` nicht erkennen. Ausserdem ist die Methode definitiv zu lang und zu komplex.

Ja, da gebe ich dir recht. Im 'Mutterprogramm' heißt das Ding auch 'parse_airmp3'. Zu lang ? Kann man für 'Gastleser' sicherlich übersichtlicher gestalten. Weiss aber nicht, ob ich persönlich das dann noch übersichtlich finde.

`checkifnumbers()` wird nirgends benutzt, aber falls es benutzt würde, was wäre der Vorteil gegenüber der `isdigit()`-Methode auf Zeichenketten? Selbst wenn man diese Methode nicht kennt *und* sonst eher in "low level"-Sprachen programmiert, ist die Funktion ziemlich gruselig. Das würde man selbst in C oder BASIC nicht so umständlich lösen.

Hat in diesem Programmauszug tatsächlich keine Bedeutung. War ein Notbehelf, weil ichs mit "Re" nicht hinbekommen hatte. Hat mal Nummern als solche erkannt und mal nicht. 'isdigit' kenn ich nicht, danke für den Tipp.

Diese ganze HTML-Parserei und das hin- und herkodieren ist ein schlechter Witz.

Das Hin- und Herkodieren ist ein Erbe aus dem eigentlichen (Haupt-)programm, welches unter wxPython läuft und mir nur so garantiert, dass auch alles auf der GUI dargestellt werden kann. Was den Witz angeht, so muss es an meiner Art von Humor liegen

``try``/``except`` ohne konkrete Ausnahme und dann auch noch mit ``pass`` ist Böse™.

Ich kann nur Ausnahmen definieren, die mir bekannt sind. Das sind sie mir aber oft nicht. Falsche Codierung? Timeout? Defekter HTML-Header? usw.
Da geht dann manchmal nur das allgemeine try/except.

Gruss, Seven

Leonidas hat geschrieben:

snakeseven hat geschrieben:Die Verwendung von Beautiful Soup fand ich da tempomäßig nicht überzeugend

Natürlich nicht, aber BeautifulSoup sehe ich eher als interessanten Hack denn als anständigen Parser. Wenn es um Geschwindigkeit geht, ist lxml das mittel der Wahl. Eigentlich ist lxml fast immer das Mittel der Wahl....

Ich wäre sogar dankbar, einen schnellen und effektiven Parser für meine Musiksuchprogrämmchen zu finden. Daher werde ich lxml auch ausprobieren.
Thx, Seven

@snakeseven: Du magst das mit den Attributen nicht schlimm finden, aber mich hat das erfolgreich davon abgehalten zu verstehen was da eigentlich vorgeht. Du benutzt die Klasse IMHO sowieso nicht richtig, das sieht alles sehr unlogisch zusammengewürfelt aus, mit willkürlichen Entscheidungen was an das Objekt gebunden wird, und was als Argumente bei Aufrufen übergeben wird. Bei "Methoden", die ein `self` bekommen, dieses aber gar nicht benutzen, sollte man einen guten Grund haben, warum die überhaupt in einer Klasse stecken. Das sieht aus wie ein Programm, welches globale Variablen verwendet, in eine Klasse verschoben, aber nicht wie eine bewusst entworfene Klasse, bei der zusammengehörige Daten und Funktionen, die darauf operieren zu einem Objekt zusammengefasst werden.

Das `src`-Argument von `loadpagesources()` wird *in* der Funktion nicht wirklich verwendet. Ausser dass es halt im Fehlerfall wieder zurückgegeben wird. Da bei allen Aufrufen der Funktion für das Argument immer die leere Zeichenkette übergeben wird, sieht das ziemlich überflüssig aus. Oder gibt's da einen tieferen Sinn, der sich mir nicht erschliesst? Das gleiche gilt für das Argument `bol`. Der Name ist IMHO übrigens echt furchtbar.

Gerade bei der Funktion `loadpagesources()` ist Dein Argument gegen Ausnahmen totaler Unsinn. Das siehst Du am Rückgabewert nur *das* etwas schiefgelaufen ist, aber all die verschiedenen Ausnahmen, eben die Details *was* genau schief lief, wird auf ein nicht sehr auskunfstfreudiges `False` eingedampft. Und ansonsten stimmt es auch nicht, denn auch mit Ausnahmen kann man im Fehlerfall ausgaben machen. Genau dazu gibt es doch ``except``, dass man auf eine Ausnahme reagieren kann. Mit zwei Vorteilen gegenüber den Fehlerwerten: 1. Man kann nicht aus versehen vergessen so einen Fehler auch zu behandeln und 2. man muss nicht direkt in der aufrufenden Funktion darauf reagieren, sondern kann die Ausnahme auch weiter oben in der Aufrufhierarchie behandeln, ohne den Fehlerwert von Hand immer weiter nach oben durchreichen zu müssen.

Wenn Dir die Ausnahmen nicht bekannst sind, dann geht natürlich etwas anderes als ``except`` ohne Ausnahme, nämlich die Klasse `Exception`. Und die auftretende Ausnahme kannst Du an einen Namen binden und dann ausgeben. So lernst Du die Ausnahmen kennen, die da auftreten. Ist Dir doch sonst so wichtig gewesen im Fehlerfall detaillierte Informationen zu bekommen und auszugeben.

Hi Black Jack,

loadpagesources() hat im Ursprungsprogramm noch weitere Aufgaben (Cookies speichern z.B.) und wird auch für eine Downloadroutine verwendet (da wird das Quellfile dann in einzelnen Blöcken geladen und triggert einen Statusbar).

Habe das alles rausgenommen, weils in diesem "Programmauszug" nicht benötigt wird (hätte stattdessen auch 'urllib2.open' in einem try/except Konstrukt verwenden können). Eigentlich ist die Methode somit überflüssig geworden und die Übergabe eines leeren Strings ist in der Tat Blödsinn (muss noch ein Relikt aus alten Tagen sein, hatte sicherlich mal Bedeutung).

Die Klasse Exception kenne ich noch nicht, werde sie mir mal anschauen.

Das mit der Klasse und den globalen Variablen siehst du richtig. Habe da noch nicht den 'goldenen Weg' gefunden, elegant und pythongerecht mit Globals umzugehen. Verzichten kann ich nicht auf sie, aber mit 'globals' möchte ich auch nicht mehr arbeiten, da ist das über eine Klasse schon wesentlich "schicker"

Werde mich erstmal mit lxml beschäftigen und dann Nachbesserungen im Gesamtstil vornehmen. Lerne ja gerne dazu !

Gruss, Seven