Zitat von
Techcrawler:
Zitat von
Luckie:
Die Kommentare sind alle Überflüssig.
Ich schreibe immer Kommentare in meinen Code, in diesem Beispiel sind die Kommentare sehr gekürzt.
Vor allem für Leute, die meinen Code später mal lesen wollen, erhöht das die Lesbarkeit.
In diesme Fall eben nicht. das:
Delphi-Quellcode:
// Songtitel extrahieren
ID3v1Infos.Songtitel := '';
for i:=4 to 33 do
ID3v1Infos.Songtitel := ID3v1Infos.Songtitel+buffer[i];
Muss ich nur kommentieren, wenn in
SongTitel nachher die Fläche eines n-Dimensionalen Oktaeders steht anstatt des SongTitels. Das Feld heißt doch schon Songtitel, was soll da anders drinne stehen?
Zitat:
Zitat von
Luckie:
Die Methode getID3v1Information würde ich private machen, das "g" groß schreiben und eine entsprechende read-Property einführen. Später kannst du dann die read-Property noch um eine write-Property erweitern, wenn man dann auch Informationen schreiben kann mit deiner Klasse.
Schreiben würde ich dann als 'setID3v1Information' schreiben, als Übergabe dann wieder Dateiname und MP3Info Record.
Nein, das machst du schon über den Konstruktor oder über die property
Filename. Der Dateiname hat an dieser Stelle nichts zu suchen, finde ich.
Zitat:
Zitat von
Luckie:
Als Rückgabetyp würde ich Boolean nehmen, wenn du keine weiteren eigenen Fehlercodes definierts.
Doch, da kommen noch Fehlercodes rein, z.B. wenn das Tag 'TAG' fehlt usw.
Würde ich auch über Exceptions lösen.
Zitat:
Zitat von
Luckie:
Warum die gloable Variabel clsMP3ID3vX?
Zum Zugriff auf die Klasse von anderen Klassen (Forms, usw.) aus.
Hä?
Deswegen einen Konstruktor. Was du machst ist gefährlich. Was wenn aus verschiedenen Units darauf zugegriffen wird? Kann sehr leicht zu inkonsistenz der Daten führen.
Zitat:
Zitat von
Luckie:
Wie wäre es mit einem Konstruktor, dem du den Dateinamen übergibst?
Hatte ich auch mal gedacht, diese Methode ist aber schneller.
Schneller? Ich glaube kaum, dass du es merkst, wenn es zwei Nanosekunden schneller geht. Der Flaschen hals hier sind die for-Schleifen und nicht das erzeugen einer Instanz mit einem Konstruktor.
Zitat:
Zitat von
Luckie:
Ich würde die Daten nicht in einem Record zurückgeben, sondern über Properties.
Könnte man auch, ich finde aber, dass ist Ansichtssache des einzelnen. Evtl. stelle ich beides zur Verfügung.
Finde ich nicht. Mit deiner Methode muss ich immer noch eine extra Variable in meinem Code deklarieren. Mit Properties brauche ich das nicht.
Zitat:
Zitat von
Luckie:
Und das:
Delphi-Quellcode:
for i:=4 to 33 do
ID3v1Infos.Songtitel := ID3v1Infos.Songtitel+buffer[i];
Sieht auch komisch aus. Wenn der Titel kürzer ist, aber das String-Ende Zeichen fehlt, schreibst du Mist in das Feld. Zumindest ein
Trim sollte noch folgen. Und könnte man da nicht
SetString nehmen oder so?
Jo, die for-Schleife ist nur ein Schnellschuß (die
unit entstand in 20 min.). Bin an der Stelle für Vorschläge offen.
Laut Spec ist das Feld _genau_ 30 Zeichen groß!
Aber nicht jeder Songtitel ist 30 zeichen lang. Was steht in den restlichen Bytes?
Deine Lösung erfordert übrigens keine Klasse. Wozu? besteht doch nur aus einer Funktion. Und so wie du Klassen nutzt, mit einer globalen Variable ist das äußerst schlecht.
Wo wird in deinem Beispiel Code eigentlich die Klasse instanziert? getID3v1Information ist doch keine Klassen-Methode. Nuimm mal die gloable Variable raus und du wirst sehen, dass du erst den Konstruktor aufrufen musst.