statt x-fache „if then else“-Abfragen eine intelligentere Lösung gesucht

[juergen]

Hallo zusammen,

wer kennt das nicht: Man fängt ein Projekt an und dann wird im Laufe der Zeit immer mehr hinten "dran gebastelt".
Ich bin nun an einem Punkt wo ich gar keine Lust mehr habe weiter zu machen, da eine bestimmte Procedure total unübersichtlich ist.

Folgende Gegebenheiten zur Umbenennung von Dateien:
Ich habe als Basis ca. 8 verschiedene MP3-Tag's (Interpret, Titel, Album usw.). Der Anwender kann momentan in einer Radio-Groupbox von ca. 10 Möglichkeiten eine Auswahl treffen, wie der Dateiname zusammengesetzt werden soll, also
a) welche MP3-Tag's überhaupt verwendet werden sollen
b) in welcher Reihenfolge der Dateiname zusammengestellt werden soll. Z.B. Interpret - Titel.mp3 oder Album - CdNr - TrackNr - Interpret - Titel.mp3

Dabei nutze ich als "Trenner" im Dateinamen jeweils " - " (Leerzeichen Bindestrich Leerzeichen).
Momentan habe ich für die gebräuchlichsten Dateinamen-Muster ein Radio-Group-Item erstellt und prüfe jedes einzelne Item über eine If then else Abfrage!
Der ganze Code wiederholt sich dann pro If-Abfrage, nur die Reihenfolge und die Anzahl der MP3-Tag's sind halt in jeder If-Abfrage anders.
Ich möchte dem Anwender keine RegEx-Ausdrücke oder dergleichen anbieten, es soll eben EINFACH und durch Lieschen Müller bedien bar sein.

Ich habe einige "nebulöse" Ansätze, wollte aber erst mal hier die Profis fragen wie sowas am besten umgesetzt werden kann um meinen völlig unübersichtlichen und sich ständig wiederholenden Code endlich richtig aufzuräumen.


Für Denkanstöße wäre ich sehr dankbar!

[lbccaleb]

Also ich würde wohl so vorgehen, dass ich die CheckBoxen in einer Case Anweisung abfrage, und für die sich wiederholenden if Klauseln, eine separate Funktion erstelle.

Mehr fällt mir im mom auch nicht ein :D

[juergen]

@lbccaleb,
danke für deine Antwort. :thumb:
Genau das war auch mein erster Ansatz. Ich hätte dann pro Checkbox eine Funktion.
Ich hoffe auf einen "dynamischere" Lösung, quasi nur eine Procedur oder dergleichen, welche bestimmte Parameter übergeben bekommt und dann alles richtig zusammenstellt....
Aber wie genau das gehen soll, da fehlt mir moementan der Ansatz.

[lbccaleb]

Da kann man bestimmt auch weiter helfen, da müsste man aber halt wohl ein wenig sehen was genau du in deiner jetzigen Funktion/Procedure machst. Dann kann man das bestimmt ein wenig zusammen fassen und ggf. in separate Funktionen packen.

[jfheins]

Auch wenn du dem Benutzer keine RegExen aufbürden willst, so kannst du das doch im Code verwenden.

Du kannst ja jedem RadioItem einen String zuweisen wie "<<I>> - <<T>> (<<Y>>)". (Zur Not über die Tag-Property und ein separates Array)
Anschließend lässt du für den konkreten Dateinamen einfach ein Suchen&Ersetzen über diesen String früberlaufen. <<I>> wird durch den Interpreten ersetzt usw.

Oder du benutzt direkt die integrierte string-Format Funktion, die kann ja auch bestimmte Indices ansprechen.

[Sir Rufo]

Baue dir doch einen Builder, der eben diese Dateinamen erstellt.

Die gewünschten Teile (Interpret, Titel, ...) und deren Reihenfolge einfach dem Builder übergeben und dann damit durch die Dateien laufen.
Für Vorbelegungen (Radiogroup) kannst du pro Eintrag eine Builder-Instanz hinterlegen und diese dann benutzen.

Delphi-Quellcode:

unit FilenameBuilder;

interface

uses
  Classes,
  Generics.Collections,
  SysUtils;

type
  TMP3Meta = record
    Interpret : string;
    Titel : string;
    Album : string;
    CdNr : Integer;
    TrackNr : Integer;
  end;

  IFilenamePart = interface
    ['{281FA130-B641-454D-8E9A-58D0C02F4C14}']
    function Build( const AData : TMP3Meta ) : string;
  end;

  TFilenamePartFactory = class
  private
    FParts : TDictionary<string, IFilenamePart>;
  public
    constructor Create;
    destructor Destroy; override;

    function GetPart( const AKey : string ) : IFilenamePart;
  end;

  TFilenameBuilder = class
  private
    FParts : TList<IFilenamePart>;
  public
    constructor Create;
    destructor Destroy; override;

    procedure AddPart( APart : IFilenamePart );
    procedure Clear;

    function GetFilename( const AData : TMP3Meta ) : string;
  end;

implementation

type
  TFilenamePart = class( TInterfacedObject, IFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; virtual; abstract;
  end;

  TInterpretPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TTitelPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TAlbumPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TCdNrPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TTrackNrPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  { TFilenameBuilder }

procedure TFilenameBuilder.AddPart( APart : IFilenamePart );
begin
  FParts.Add( APart );
end;

procedure TFilenameBuilder.Clear;
begin
  FParts.Clear;
end;

constructor TFilenameBuilder.Create;
begin
  inherited Create;
  FParts := TList<IFilenamePart>.Create;
end;

destructor TFilenameBuilder.Destroy;
begin
  FParts.Free;
  inherited;
end;

function TFilenameBuilder.GetFilename( const AData : TMP3Meta ) : string;
var
  LBuilder : TStringBuilder;
  LIdx :     Integer;
begin
  LBuilder := TStringBuilder.Create;
  try
    for LIdx := 0 to FParts.Count - 1 do
      begin
        if LIdx > 0
        then
          LBuilder.Append( '-' );

        LBuilder.Append( FParts[LIdx].Build( AData ) );
      end;

    Result := LBuilder.ToString;

  finally
    LBuilder.Free;
  end;
end;

{ TInterpretPart }

function TInterpretPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := AData.Interpret;
end;

{ TTitelPart }

function TTitelPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := AData.Titel;
end;

{ TAlbumPart }

function TAlbumPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := AData.Album;
end;

{ TCdNrPart }

function TCdNrPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := IntToStr( AData.CdNr );
end;

{ TTrackNrPart }

function TTrackNrPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := IntToStr( AData.TrackNr );
end;

{ TFilenamePartFactory }

constructor TFilenamePartFactory.Create;
begin
  inherited Create;
  FParts := TDictionary<string, IFilenamePart>.Create;

  FParts.Add( 'Interpret', TInterpretPart.Create );
  FParts.Add( 'Titel', TTitelPart.Create );
  FParts.Add( 'Album', TAlbumPart.Create );
  FParts.Add( 'CdNr', TCdNrPart.Create );
  FParts.Add( 'TrackNr', TTrackNrPart.Create );
end;

destructor TFilenamePartFactory.Destroy;
begin
  FParts.Free;
  inherited;
end;

function TFilenamePartFactory.GetPart( const AKey : string ) : IFilenamePart;
begin
  Result := FParts[AKey];
end;

end.

Das sieht zwar wild aus, ist aber in der Anwendung relativ zahm

Delphi-Quellcode:

program FilenameBuilderTest;

{$APPTYPE CONSOLE}
{$R *.res}

uses
  System.SysUtils,
  FilenameBuilder in 'FilenameBuilder.pas';

var
  PartFactory : TFilenamePartFactory;

procedure Test;
var
  LFilenameBuilder : TFilenameBuilder;
  LData :            TMP3Meta;
begin

  LData.Interpret := 'Testinterpret';
  LData.Titel     := 'Testtitel';
  LData.Album     := 'TestAlbum';
  LData.CdNr      := 1;
  LData.TrackNr   := 10;

  LFilenameBuilder := TFilenameBuilder.Create;
  try

    LFilenameBuilder.AddPart( PartFactory.GetPart( 'TrackNr' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'CdNr' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'Album' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'Interpret' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'Titel' ) );

    WriteLn( LFilenameBuilder.GetFilename( LData ) );

    LFilenameBuilder.Clear;

    LFilenameBuilder.AddPart( PartFactory.GetPart( 'CdNr' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'TrackNr' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'Interpret' ) );
    LFilenameBuilder.AddPart( PartFactory.GetPart( 'Titel' ) );

    WriteLn( LFilenameBuilder.GetFilename( LData ) );

  finally
    LFilenameBuilder.Free;
  end;

end;

begin
  try

    PartFactory := TFilenamePartFactory.Create;
    try

      Test;

    finally
      PartFactory.Free;
    end;

  except
    on E : Exception do
      WriteLn( E.ClassName, ': ', E.Message );
  end;

  ReadLn;

end.

[juergen]

@jfheins,
Danke für deine Idee. :thumb: Ich denke so werde ich es nun erstmal umsetzen.

@Sir Rufo,
Vielen Dank für deine Mühe!:thumb:
Da ich für dieses Projekt noch D2007 nutze wo es die Generics.Collections nicht gibt, kann ich das momentan so nicht umsetzen.
Einiges von deinem Code habe ich verstanden, einiges aber auch nicht.:oops:
Deine Mühe war nicht umsonst, da ich versuchen möchte deine Art der Umsetzung zu verstehen!

Was ich momentan gar nicht verstehe ist folgendes:

Delphi-Quellcode:

  IFilenamePart = interface
     ['{281FA130-B641-454D-8E9A-58D0C02F4C14}']
     function Build( const AData : TMP3Meta ) : string;
   end;

In der Hilfe zu interface steht, dass eine mit einem Interface-Typ deklarierte Variable Instanzen jeder Klasse referenzieren kann.
Ich denke dass IFilenamePart diese Variable ist. Aber wozu dient dieser seltsame Zahlen/Buchstaben-String ({281FA130-B641-454D-8E9A-58D0C02F4C14})?
Ist das eine Vorbelegung der Variable, damit diese von Anbeginn nicht leer ist?

Grüße Jürgen

[stahli]

Als generelle Anmerkung noch folgendes:

Man kann in einer Funktion statt vieler if then else ggf. auch mit Exit arbeiten:

Delphi-Quellcode:

function Test: Integer;
begin
  if a then
    Exit(1);
  if b then
    Exit(2);
  if c then
    Exit(3);
end;

Wenn zum Schluss noch etwas erledigt werden muss hilft try finally:

Delphi-Quellcode:

function Test: Integer;
begin
  try
    if a then
      Exit(1);
    if b then
      Exit(2);
    if c then
      Exit(3);
  finally
    Print(Result);
  end;
end;

Das ist m.E. etwas schlanker als viele if then else.
(Wobei manche Entwickler Exit eher ablehnen.)

[Sir Rufo]

Zitat:

Zitat von juergen (Beitrag 1216985)

Was ich momentan gar nicht verstehe ist folgendes:

Delphi-Quellcode:

  IFilenamePart = interface
     ['{281FA130-B641-454D-8E9A-58D0C02F4C14}']
     function Build( const AData : TMP3Meta ) : string;
   end;

In der Hilfe zu interface steht, dass eine mit einem Interface-Typ deklarierte Variable Instanzen jeder Klasse referenzieren kann.

Ja, jeder Klasse kannst du auch noch ein oder mehrere Interfaces aufdrücken, allerdings muss die Klasse die Methoden vom Interface auch implementieren.

Zitat:

Zitat von juergen (Beitrag 1216985)

Ich denke dass IFilenamePart diese Variable ist.

Nö, das ist die Deklaration des Interfaces

Zitat:

Zitat von juergen (Beitrag 1216985)

Aber wozu dient dieser seltsame Zahlen/Buchstaben-String ({281FA130-B641-454D-8E9A-58D0C02F4C14})?

Der seltsame Zahlen/Buchstaben-String ist eine GUID und stellt einfach nur eine eindeutige Kennzeichnung für das Interface dar.
Die sollte da sein, ist aber erstmal nicht weiter von Belang (tu so, als ob die nicht da wäre ;))

Zitat:

Zitat von juergen (Beitrag 1216985)

Ist das eine Vorbelegung der Variable, damit diese von Anbeginn nicht leer ist?

Nein :)

Ohne Generics geht das auch - ist aber nicht so schön :)

Mal sehen, ob ich das so auf die Schnelle hingezaubert bekomme ...

[Sir Rufo]

Sodele hier ohne Generics

Delphi-Quellcode:

unit FilenameBuilder;

interface

uses
  Classes,
  SysUtils;

type
  TMP3Meta = record
    Interpret : string;
    Titel : string;
    Album : string;
    CdNr : Integer;
    TrackNr : Integer;
  end;

  IFilenamePart = interface
    ['{281FA130-B641-454D-8E9A-58D0C02F4C14}']
    function Build( const AData : TMP3Meta ) : string;
  end;

  TFilenameBuilder = class
  private
    FParts : TInterfaceList;
  public
    constructor Create;
    destructor Destroy; override;

    procedure AddPart( APart : IFilenamePart );
    procedure Clear;

    function GetFilename( const AData : TMP3Meta ) : string;
  end;

type
  TFilenamePart = class( TInterfacedObject, IFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; virtual; abstract;
  end;

  TInterpretPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TTitelPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TAlbumPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TCdNrPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

  TTrackNrPart = class( TFilenamePart )
  protected
    function Build( const AData : TMP3Meta ) : string; override;
  end;

implementation

{ TFilenameBuilder }

procedure TFilenameBuilder.AddPart( APart : IFilenamePart );
begin
  FParts.Add( APart );
end;

procedure TFilenameBuilder.Clear;
begin
  FParts.Clear;
end;

constructor TFilenameBuilder.Create;
begin
  inherited Create;
  FParts := TInterfaceList.Create;
end;

destructor TFilenameBuilder.Destroy;
begin
  FParts.Free;
  inherited;
end;

function TFilenameBuilder.GetFilename( const AData : TMP3Meta ) : string;
var
  LBuilder : TStringBuilder;
  LIdx :     Integer;
begin
  LBuilder := TStringBuilder.Create;
  try
    for LIdx := 0 to FParts.Count - 1 do
      begin
        if LIdx > 0
        then
          LBuilder.Append( '-' );

        LBuilder.Append( ( FParts[LIdx] as IFilenamePart ).Build( AData ) );
      end;

    Result := LBuilder.ToString;

  finally
    LBuilder.Free;
  end;
end;

{ TInterpretPart }

function TInterpretPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := AData.Interpret;
end;

{ TTitelPart }

function TTitelPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := AData.Titel;
end;

{ TAlbumPart }

function TAlbumPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := AData.Album;
end;

{ TCdNrPart }

function TCdNrPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := IntToStr( AData.CdNr );
end;

{ TTrackNrPart }

function TTrackNrPart.Build( const AData : TMP3Meta ) : string;
begin
  Result := IntToStr( AData.TrackNr );
end;

end.

und die kleine Demo

Delphi-Quellcode:

program FilenameBuilderTest;

{$APPTYPE CONSOLE}
{$R *.res}

uses
  System.SysUtils,
  FilenameBuilder in 'FilenameBuilder.pas';

procedure Test;
var
  LFilenameBuilder : TFilenameBuilder;
  LData :            TMP3Meta;
begin

  LData.Interpret := 'Testinterpret';
  LData.Titel     := 'Testtitel';
  LData.Album     := 'TestAlbum';
  LData.CdNr      := 1;
  LData.TrackNr   := 10;

  LFilenameBuilder := TFilenameBuilder.Create;
  try

    LFilenameBuilder.AddPart( TTrackNrPart.Create );
    LFilenameBuilder.AddPart( TCdNrPart.Create );
    LFilenameBuilder.AddPart( TAlbumPart.Create );
    LFilenameBuilder.AddPart( TInterpretPart.Create );
    LFilenameBuilder.AddPart( TTitelPart.Create );

    WriteLn( LFilenameBuilder.GetFilename( LData ) );

    LFilenameBuilder.Clear;

    LFilenameBuilder.AddPart( TCdNrPart.Create );
    LFilenameBuilder.AddPart( TTrackNrPart.Create );
    LFilenameBuilder.AddPart( TInterpretPart.Create );
    LFilenameBuilder.AddPart( TTitelPart.Create );

    WriteLn( LFilenameBuilder.GetFilename( LData ) );

  finally
    LFilenameBuilder.Free;
  end;

end;

begin
  try

    Test;

  except
    on E : Exception do
      WriteLn( E.ClassName, ': ', E.Message );
  end;

  ReadLn;

end.