Codeprüfung/-vorschläge | Thema: MP3, ID-Infos

[Luckie]

Die Kommentare sind alle Überflüssig.

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.

Als Rückgabetyp würde ich Boolean nehmen, wenn du keine weiteren eigenen Fehlercodes definierts.

Warum die gloable Variabel clsMP3ID3vX?

Wie wäre es mit einem Konstruktor, dem du den Dateinamen übergibst?

Ich würde die Daten nicht in einem Record zurückgeben, sondern über Properties.

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?

Delphi-Quellcode:

type
  TMp3Info = class(TObject);
    private
      procedure SetFilename(const Filename: String);
      function GetFilename: String;
      function GetMp3Info: Boolean;
      function GetTitle: String;
      function GetArtist: String;
      // ..
    public
      procedure Create(const Filename: String); override;
      procedure Create; override;
      property Filename: String read GetFilename write SetFilename;
      property Title: String read GetTitle;
      property Artist: String read GetArtist;
      // ...
  end;

So. Zwei Konstruktoren, damit, wenn man meherer Datein abfragen will, nicht immer die Instanz neu anlegen muss. Dafür brauchen wir auch die Property Filename. SetFilename sollte eine Exception auslösen, wenn die Datei nicht existiert. GetMp3Info sollte auch eine Exception auslösen, wenn die ID3 Informationen nicht gelesen werden können und auch noch mal, überprüfen, ob die Datei immer noch da ist und dann eventuell auch eine Exception auslösen. (Kann ja sein, dass sie nicht mehr da ist, weil sie gelöscht, umbenannt wurde oder das Laufwerk nicht mehr verfügbar ist.) Insofern könnte GetMp3Info auch eine Prozedur sein.

Das wäre meine Lösung. Muss nicht die Meinung aller treffen, aber so sieht es für mich sauber und schlüssig aus.