Hi,
ich denke da gibt es wirklich ein paar Punkte, die du dir noch mal überlegen solltest. Ok, sehe gerade den roten Kasten und möchte die Kritik dort ergänzen.
An sich solltest du wirklich überlegen, ob du hier ein Record verwenden möchtest. Deine TMP3ID3vX ist ja schon mal eine Klasse, da sehe ich dann wenig Sinn nicht komplett auf die
OOP zu setzen. Das ganze bringt dir schon mal mindestens den Vorteil, dass du dich weniger um die Speicherverwaltung kümmern musst. Zudem kannst du das var beim übergeben weglassen, denn hier würde aut. eine Referenz übergeben werden.
Was völlig fehlt ist bei dir jegliches Fehlermanagment. Das wurde zwar schon gesagt, aber auffällig ist, dass du hier davon ausgehst, dass du einen gültigen Pfad bekommst. Was aber wenn der nicht mehr gültig ist (jmd. wählt die Datei aus und verschiebt die kurze Zeit später, löscht sie, ...). Da solltest du auf jeden Fall mit einem Ressourcenschutzblock arbeiten und an sich immer dafür sorgen, dass alle erzeugten Ressourcen immer sicher frei gegeben werden.
Ganz schlecht ist auch die Art und Weise, wie du hier mit den Strings arbeitest. Strings sind eine sehr besondere Struktur, du solltest sie immer als eine Klasse mit statischem impliziten Konstruktor betrachten. Anders gesagt, jede Änderung des Strings bring etwas Overhead mit sich, da größerer Speicher alloziert wird, der alte String dort hineinkopiert und um den neuen String erweitert wird. Besser als eine Schleife und ein s + x ist es, wenn du hier einfach copy verwendest und direkt den entsprechenden Bereich in einem Rutsch kopierst.
Dann solltest du auch noch schauen, um welche Version von ID3v1 es sich handelt, wenn ich mich da richtig erinnere, gab es da zwei und die solltest du auch unterscheiden (einmal mit und einmal ohne Genre?).
Zu guter letzt würde ich jetzt noch sagen, dass dein Klassenname TMP3ID3vX gaaaanz schlecht gewählt ist. Erstens solltest du hier nicht einfach nur Großbuchstaben (sehen wir vom x ab) verwenden. Nur der Anfangsbuchstabe eines Wortes, aber auch eines Akronyms, etc. sollte groß geschrieben werden, das macht es viel leichter möglich zu erkennen, worum es sich hier handelt. Der Name TMp3Id3vX wäre aber immernoch schlecht gewählt. vX? Da würde ich jetzt nicht davon ausgehen, dass ich ein TId3v1 bekomme, sondern die Auswahl habe (bei dir noch nicht berücksichtigt, geht aber nicht aus dem Namen hervor).
Gruß Der Unwissende
Zitat von
Techcrawler:
Solange die Kommentare keine Zeitliche verzögerung bedeuten oder das compilierte Programm vergrößern, können die drinbleiben (das ist ja eh ansichtssache des einzelnen Entwicklers).
Äh, Kommentare werden immer vom Compiler gelöscht (sonst wären das ja Annotationen
)
Zitat von
Techcrawler:
Zitat von
Luckie:
Warum die gloable Variabel clsMP3ID3vX?
Zum Zugriff auf die Klasse von anderen Klassen (Forms, usw.) aus.
Ganz schlechter Weg! Schau dir mal das Singleton Pattern an, besserer Weg! Oder halt erzeugen wenn man es benutzt und danach wieder frei geben (dann können auch mehrere gleichzeitig arbeiten.
Zitat von
Techcrawler:
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.
Von wie vielen ms reden wir hier?