Methoden in abgeleiteten Klassen ggf. einschränken

[Gausi]

Zitat von hanvas:

Im Grunde ist die Unterscheidung zwischen meiner Lösung und der Verwendung von Interfaces reine Semantik. Sie sind äquivalent.

Nicht ganz. Denn ich habe keine lineare Klassenstruktur, sondern gewissermaßen eine quadratische. Ich brauche für ein TagItem einmal die Unterscheidung nach (A) ID3v2TagItem, TOggVorbisTagItem, TApeTagItem (...). Darin gibt es dann jeweils Methoden, die die Daten aus einem FileStream (mp3-Datei, ogg-Datei, ape-Datei, ...) laden und passend aufbereiten.
Und ich brauche (in dem Ansatz) eine Unterscheidung nach (B) TTextTagItem, TPictureTagItem, TBinaryTagItem (...), die die Daten darin für die Anzeige vorbereiten bzw. über sinnvolle Getter und/oder Setter bereitstellen.

Wenn ich nur mit Klassen arbeite, müsste ich im Programmcode mangels Mehrfachvererbung dann sehr viele Typen (A*B viele) abfragen und einzeln behandeln. Mit Interfaces und Support könnte ich mit einer Abfrage alle TextTagItems behandeln - egal ob sie nun in einem ID3v2TagItem, TOggVorbisTagItem oder TApeTagItem implementiert sind. Das macht dann schon einen Unterschied.

Zitat von himitsu:

Modern baut man sich natürlich statt dem GetAllTagItems einen Enumerator,

Das wäre dann noch ein anderes Thema. Da ich aber auch noch "von damals" Compilerschalter für "nicht-Unicode" Versionen von Delphi drinhabe, werde ich mir diesen modernen Kram vermutlich sparen.

[Kas Ob.]

I would go with something like TMetaDataAudioFactory with half fully-abstracted base class that support everything.
The base with have everything you could use or has been implemented but in two ways, like for each type there is a method like

Code:

function LyricsSupported : Boolean;

This will indicate that LyricsCount and LyricsItems are possible to use. (In an imaginary case where there is multiple lyrics in multiple language)

In the base/abstract class LyricsSupported is an abstract method, meaning all the derived class must implement this one, this will force you to not forget about them.
While in that base methods like LyricsCount and LyricsItems will raise an exception this will enforce you and the users of your library to check LyricsSupported before use, and will remove the need to implement in the derived and specific MetaData parsers, simply override the base classes to prevent raising exceptions.

for the end user/developer the use is straight forward, call PictureSupported then use Picture, or Picture and PictureCount.... and may be PictureMaxCount, that depends on what you see fit for each file type.

[Gausi]

Danke für die Anregungen, einige davon habe ich mit berücksichtigt. Das mit den Interfaces lass ich aber bleiben - das würde nicht viel zur Übersicht beitragen, und letztlich auch nicht alle Probleme lösen.

In jeder TagItem-Klasse muss bei den verschiedenen Set/Get-Methoden überprüft werden, ob dieser Aufruf sinnvoll ist. Dafür ist der "TagContentType" zuständig, der in den unterschiedlichen Tag-Formaten mehr oder weniger viele Werte annehmen kann, und der mehr oder weniger kompliziert zu bestimmen ist. Was genau "Sinnvoll" heißt, kann dabei vom Anwender der Lib mehr oder weniger streng ausgelegt werden - zumindest beim Thema "Text", was vor allem beim ID3v2-Tag wichtig ist. Über den optionalen Parameter "TextMode" kann der Anwender steuern, ob wirklich nur Text ausgegeben werden soll, oder ob man den Begriff etwas weiter fassen will. Also z.B. auch den "Text-Teil" von Text-Frames mit weiteren (meist uninteressanten) Metadaten haben will, oder ob man sogar eine (natürlich nicht 100% akkurate) Darstellung von binären Daten haben will.

Text auslesen und schreiben geht sieht beim ID3v2-Tag jetzt so aus:

Delphi-Quellcode:

function TID3v2Frame.GetText(TextMode: TTextMode = tmReasonable): UnicodeString;
var
  ct: TTagContentType;
  Lang: AnsiString;
  Description: UnicodeString;
begin
  result := '';
  if not fParsable then
    exit
  else begin
    ct := TagContentType;
    if ct = tctText then
      result := GetConvertedUnicodeText(1, length(fData) - 1, ByteToTextEncoding(fData[0]))
    else begin
      case ct of
        tctComment,
        tctLyric: begin
          if TextMode in [tmReasonable, tmForced] then
            result := GetCommentsLyrics(Lang, Description);
        end;

        tctURL: begin
          if TextMode in [tmReasonable, tmForced] then
            result := UnicodeString(GetURL);
        end;

        tctUserText: begin
          if TextMode in [tmReasonable, tmForced] then
            result := GetUserText(Description);
            if Description <> '' then
              result := '(' + Description + ') ' + result;
        end;

        tctUserURL: begin
          if TextMode in [tmReasonable, tmForced] then
            result := UnicodeString(GetUserDefinedURL(Description));
        end;

        tctPicture,
        tctBinary,
        tctPopularimeter,
        tctPrivate,
        tctUnknown: begin
          if TextMode = tmForced then
            result := ByteArrayToString(fData);
        end

      else
        result := '';
      end;
    end;
  end;
end;

function TID3v2Frame.SetText(aValue: UnicodeString; TextMode: TTextMode = tmReasonable): Boolean;
var
  ct: TTagContentType;
  dummyA, curLang: AnsiString;
  dummy, curDesc: UnicodeString;
begin
  result := False;
  ct := TagContentType;
  if ct = tctText then
    result := DoSetText(aValue) // always true :)
  else begin
    // Try to set "reasonable" text frames
    // Mode "tmForced" for binary or unknown frames doesn't make any sense here, it will just make the frame invalid
    if TextMode in [tmReasonable, tmForced] then begin
      case ct of
        tctComment,
        tctLyric: begin
          dummy := GetCommentsLyrics(curLang, curDesc); // parameters are OUT parameters ;-)
          result := SetCommentsLyrics(curLang, curDesc, aValue);
        end;

        tctURL: begin
          result := SetURL(AnsiString(aValue));
        end;

        tctUserText: begin
          dummy := GetUserText(curDesc);
          result := SetUserText(curDesc, aValue);
        end;

        tctUserURL: begin
          dummyA := GetUserDefinedURL(curDesc);
          result := SetUserDefinedURL(curDesc, AnsiString(aValue));
        end;
      else
        result := False;
      end;
    end;
  end;
end;

Im Testprogramm reichen dann wenige Zeilen, um alle enthaltenen Tags anzuzeigen (sieht dann so aus wie im Screenshot)

Delphi-Quellcode:

procedure TMainFormAWB.RefillTagItems;
var
  i: Integer;
  TagItems: TTagItemList;
begin
  TagItems := TTagItemList.Create;
  try
    AudioFile.GetTagList(TagItems, [tctAll]);
    for i := 0 to TagItems.Count - 1 do
      AddTagItem(cTagTypes[TagItems[i].TagType] , TagItems[i].Key, TagItems[i].GetText(tmForced));
      // AddTagItem: ListViewTags.Items.Add; etc. pp.
  finally
    TagItems.Free;
  end;
end;

Ist nur einiges an Aufwand, alle Klassen entsprechend anzupassen. Aber ich denke, der Aufwand lohnt sich. Die Verwendung der Lib wird dadurch deutlich übersichtlicher. In ein paar wenigen Fällen schmeiße ich auch eine Exception, aber an diese Stellen sollte der Entwickler in aller Regel nicht rankommen.