Methoden in abgeleiteten Klassen ggf. einschränken

[Gausi]

Das Problem dabei wäre, dass ich dann (abgesehen von TGenericTagItem) keinen sinnvollen gemeinsamen Vorfahren für die TagItems in den jeweiligen MetaDaten-Formaten nutzen kann. Denn die Unterscheidung Text/Bild/ExtendendText/Binary/... gibt es jeweils für ID3v2Items, Apev2Items, VorbisCommentItems, ...

Dann müsste ich wieder im eigentlichen Programmcode für jedes Format anders casten bzw. den Typ checken, um "GetText" aufrufen zu können. Und genau davon möchte ich ja weg.

[Uwe Raabe]

Wie wäre es denn mit Interfaces?

Delphi-Quellcode:

type
type
  ITextTag = interface
    ['{1EE16FB9-1BDD-4BDA-86DE-2A09C314E845}']
    function GetText: string; 
    procedure SetText(const Value: string); 
    property Text: string read GetText write SetText;
  end;

  IImageTag = interface
    ['{99D31F17-7D69-47FA-98BE-F1F13CB02E39}']
    function GetImage: TGraphic; 
    procedure SetImage(const Value: TGraphic); 
    property Image: TGraphic read GetImage write SetImage;
  end;

  IMetaDataTag = interface
  ['{F09ACD7B-CDC0-4548-A248-1F2DE6B7EE41}']
    function GetMetaData: string; 
    procedure SetMetaData(const Value: string); 
    property MetaData: string read GetMetaData write SetMetaData;
  end;

Dann kannst du über Supports abfragen, welches Feature ein TagItem unterstützt oder nicht.

[Gausi]

Zitat von Uwe Raabe:

Wie wäre es denn mit Interfaces?

Mit so einer Antwort habe ich gerechnet . Der Gedanke kam mir auch schon, und das wäre vermutlich aus Perspektive der Objekt-Orientierung die sauberste Lösung. Dann müsste ich mich während des Auslesens der Metadaten aus einer Datei vor Erstellung des jeweiligen TTagItems entscheiden, welche konkrete Klasse ich da instanziieren will/muss. Dann ist schon bei der Erstellung definiert, was das Objekt "kann".

Mit Interfaces und Supports() kann ich dann bei der Anwendung direkt eine ganze Reihe in der "2D-Klassen-Matrix" abfrühstücken, und muss nicht jede einzelne Klasse testen. Das wäre auf jeden Fall ein Fortschritt zur aktuellen (nicht-)Lösung.

In meinem ersten Gedanken (Überprüfung in GetText bzw. SetText) wird diese Entscheidung erst später getroffen, indem die TagItem-Klasse checkt, ob es sinnvoll ist, diese oder jene geerbte Methode auszuführen.

Ob Interfaces für meine Anwendungsfälle ideal wären, muss ich mir nochmal durch den Kopf gehen lassen.

Eine Methode "GetText" ist nämlich manchmal auch für binäre Daten reizvoll (nicht druckbare Zeichen dann durch "." oder so ersetzt, wie bei Hex-Editoren auch). Und bei den "erweiterten Text-Items" müsste ich mir auch noch überlegen, ob ich dafür das Interface "GetText" bereitstelle, oder nicht. (Lyrics enthalten z.B. im ID3v2Tag nicht nur den eigentlichen Liedtext, sondern z.B. auch noch ein Feld für die Sprache. Da enthält quasi ein TTagItem selbst wieder Metadaten )
Mit meiner zuerst angedachten Lösung könnte ich das Verhalten vom Anwender der Library über einen zusätzlichen Parameter "TextMode" steuern lassen, der z.B. "strict", "reasonable" oder "forced" sein kann. Im letzteren Fall (forced) könnte der Anwender dann Unsinn machen (zumindest mit SetText), wenn er will (oder genau weiß, was er tut).

@hanvas: von genau diesen (vielen) Fallunterscheidungen im eigentlichen Programmcode möchte ich ja weg. Ich möchte quasi etwas mehr Programmlogik vom Anwendungscode in die Library verschieben. Aktuell ist das noch wirrer, da die "GetAllTagItems"-Methode in jeder Tag-Klasse anders heißt, und auch nicht immer den gleichen Listentyp haben will (TObjectList, TList, TStringList).

Aber danke für die Anregungen - das hilft ja oft schon, um sich selbst besser klar zu werden, was man eigentlich will.

[hanvas]

Zitat von Gausi:

Zitat von Uwe Raabe:

Wie wäre es denn mit Interfaces?

@hanvas: von genau diesen (vielen) Fallunterscheidungen im eigentlichen Programmcode möchte ich ja weg. Ich möchte quasi etwas mehr Programmlogik vom Anwendungscode in die Library verschieben.

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

Zitat:

Dann müsste ich mich während des Auslesens der Metadaten aus einer Datei vor Erstellung des jeweiligen TTagItems entscheiden, welche konkrete Klasse ich da instanziieren will/muss.

So wars gedacht. Aber egal ob Interfaces oder Objekte, in jedem Fall könntest Du die Fallunterscheidung in die Methode GetAllTagItems verschieben wenn du einen zweiten Paramter als Filter einführst :

Delphi-Quellcode:

 TagList := TTagItemList.Create;
 try
   aAudioFile.GetAllTagItems(TagList, TTextIDTag);
   for var tag : TTextIDTag in TagList do
   begin
     text := tag.GetText()   
     machWasMitText(text)
     tag.SetText(Text);
   end;
 finally
   TagList.Free;
 end;

das würde Dir dann auch

Zitat:

Eine Variante procedure GetAll_Text_TagItems(dest: TTagItemList); wird es natürlich auch geben

ersparen. Du hättest es dann ja schon.

cu Ha Jo

[himitsu]

Modern baut man sich natürlich statt dem GetAllTagItems einen Enumerator,
aber OK, ein Array als Result geht och, und sieht in diesem Fall von der Verwendung her gleich aus.

for var Tag in aAudioFile.Tags do

[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.

[hanvas]

Zitat von Gausi:

Das Problem dabei wäre, dass ich dann (abgesehen von TGenericTagItem) keinen sinnvollen gemeinsamen Vorfahren für die TagItems in den jeweiligen MetaDaten-Formaten nutzen kann. Denn die Unterscheidung Text/Bild/ExtendendText/Binary/... gibt es jeweils für ID3v2Items, Apev2Items, VorbisCommentItems, ...

Dann müsste ich wieder im eigentlichen Programmcode für jedes Format anders casten bzw. den Typ checken, um "GetText" aufrufen zu können. Und genau davon möchte ich ja weg.

Warum machst Du das Verhalten nicht einfach konfigurierbar und überlässt die Wahl (nichts machen, den "Fehler" loggen, eine Exception auslösen, den "Fehler" in einer Variablen speichern und mittels einer Methode wie getLastErrorCode auslesen usw.) nicht einfach den Benutzer deiner Lib.

Aber abgesehen davon. Wenn ich Dein Problem richtig verstehe dann hast Du nach:

Delphi-Quellcode:

aAudioFile := AudioFileFactory.CreateAudioFile(aFileName);
aAudioFile.ReadFromFile(aFileName);
aAudioFile.Title := EditTitle.Text;
aAudioFile.UpdateFile;

doch alle Informationen die Du brauchst. Kanst Du nicht beispielsweise AudioFileFactory um eine Methode erweitern die zu jedem Tag die richtige Klasse (oder alternativ Interface) registriert, und nutzt diese Informationen in der Methode GetAllTagItems um die richtigen Klassen zu instanzieren.

Delphi-Quellcode:

type TGenericIDTag = class ...

     TTagClass = class of TGenericIDTag;

     TTextIDTag = class( TGenericIDTag )
     end;

     TImageIDTag = class(TGenericIDTag)
     end;

     TAudioFile = class()
     private
       FFactory :  TAudioFileFactory
     public
       procedure GetAllTagItems(destList: TTagItemList); 
     end;

     TAudioFileFactory = class()
     public 
       function  CreateAudioFile(const aFileName : String) : TAudioFile;       
       procedure RegisterTag(const aIdentifier : String; aTag : TTagClass);
     end;

dann hättest Du alle Informationen um beim Aufruf von GetAllTagItems eine Liste mit den "richtigen" Objekten zu konstruieren.

Delphi-Quellcode:

 TagList :=  TTagItemList.Create;
 try
   aAudioFile.GetAllTagItems(TagList);
   for var tag : TGenericIDTag in TagList do
    begin
     if tag is TTextIDTag then
      begin
        ....
      end;
    end;
 finally
 end;

Ich hofffe ich habe mich verständlich ausgedrückt.

hth Ha Jö