Code Optimierung gesucht

[Martin W]

Zitat von glotzer:

An sich spricht doch auch nix dagegen dass ein Auto in mehreren Listen gleichzeitig ist oder? Du kannst ja deine einzelnen Listen beibehalten und eben zusätzlich noch eine große gemeinsame Liste führen.

Ja, hab es jetzt mit Properties gelöst. Beim Zugriff auf die Variable wird die Meta-Liste jeweils neu zusammmengebaut.

[Bjoerk]

Hast du die Idee von Markus mit dem Array probiert?

Delphi-Quellcode:

TAutoHersteller = (ahVW, ahMBW, ahBenz)

TAutoList = class(TObjectList)

TAutos = array[TAutoHersteller] of TAutoList;

for AutoHersteller := Low(TAutoHersteller) to High(TAutoHersteller) do
 for I := 0 to FAutos[AutoHersteller].Count - 1 do
   if FAutos[AutoHersteller].Items[I].NameOfCar = aCar then

[Dejan Vu]

Also ich würde wirklich einen einzigen Container für alle Autos verwenden. Falls nur über Autos eines bestimmten Typen iteriert werden soll, dann spendiert man dem Container eine Methode/Funktion, mit der man durch alle Autos eines bestimmten Typen iterieren kann.

Delphi-Quellcode:

Type
  myCarContainer : TCarContainer;

Begin
  myCarContainer := TCarContainer.Create;
  myCarContainer.Add(THerstellerA.Create);
  ...
  ...

Var
  myIterator : TCarIterator<THerstellerA>;
  allIterator : TCArIterator<TAuto>;
begin
  myIterator := myCarContainer.GetCars<THerstellerA>;
  while myIterator.Next do
    DoSomethingWith (myIterator.Current);
  ...
  allIterator := myCarContainer.GetCars<TAuto>;
  while allIterator.Next do
    myIterator.Current.Color := NeueFarbe;
  ...

Man könnte sich vom Container auch gleich die ganze Liste geben lassen, aber dann hätte man die Instanzen redundant gespeichert, was unnötig ist.

Natürlich soll man den bestehenden Code nicht umschreiben, es kann also wirklich sinnvoll sein, die Listen aus dem Container zu extrahieren, um die betreffenden Codestellen nicht zu sehr umschreiben zu müssen. Allerdings glaube ich, das man hier mit Refaktoring sehr viel vereinfachen kann, denn irgendwie scheinen die ganzen Listen doch eh nur immer wieder durchlaufen zu werden. Und das geht auch sehr elegant mit einer ForEach-Methode:

Delphi-Quellcode:

...
myCarContainer.ForEach<THerstellerA>(ChangeColor);
...

Function ChangeColor (item : THerstellerA) : Boolean;
Begin
  item.Color := ColorToChange;
  Result := True; // False würde den 'ForEach' Operator abbrechen
End;

Das kann man noch beliebig weiterspinnen. Wichtig ist hier, das man zuerst die Funktionalität definiert, d.h. das Interface und das dann implementiert. Und das Interface richtet sich eben u.a. daran, möglichst wenig am Code kaputt zu optimieren.

[alda]

Unabhängig von den Listen solltest Du auf jeden Fall auch deine Klasen-Struktur überdenken - aber ich vermute, dass Dein Code schon Älter ist und eine Anpassung sehr aufwendig ist. Hier ein paar Ideen:

• auf keinen Fall mehrere Listen - damit ist jede verwendete Codestelle anzupassen, sofern ein neuer Hersteller hinzukommt. (da schließ ich mich dem Rest an)
• Warum von TComponent ableiten? Egal für welchen aktuellen Zweck erachte ich das als nicht sinnvoll.
• Ableitungen pro Autohersteller halte ich im allgemeinen auch nicht für sinnvoll
• die Ableitungen pro Hersteller scheinen aufgrund der als bool (vereinfachten) Eigenschaften vorgenommen worden zu sein, sollten wiederum aber auch eigene Klassen sein (ich nenne Sie mal TAusstattung), da die Vererbung die Flexibilität hier stark eingrenzt.
• Einsatz von Interfaces für bestmögliche Entkopplung und Testbarkeit

[Dejan Vu]

Zitat von alda:

...

• Ableitungen pro Autohersteller halte ich im allgemeinen auch nicht für sinnvoll

...

Sind sie aber. Alles andere (Enum/ Eigenschaft 'Fahrzeugtyp') bedeutet auch: Verletzung des OCP, d.h. die 'Auto-Klasse' müsste jedes mal angefasst werden, wenn eine neue Type mit spezifischen Eigenschaften hinzukommt. Hast Du dagegen pro Hersteller eine eigene Klasse, bleiben alle existierenden Klassen beim Hinzufügen einer neuen Klasse unangetastet, was den Code wesentlich robuster, d.h. stabiler macht.

Nur wenn es je Hersteller garantiert keine individuellen Eigenschaften gäbe (was hier nicht er Fall ist), muss man keine einzelne Klassen bilden.

EDIT: Habe deinen Post gerade noch einmal gelesen: Du meinst, ein TAuto, aber je Hersteller unterschiedliche Ausstattungen? Das wäre eine Möglichkeit, wobei diese Architektur impliziert, das man den Hersteller wechseln kann. Weiterhin müsste man die Eigenschaft 'Ausstattung' jedes Mal casten, um auf spezifische Eigenschaften eines Herstellers zu gelangen.
Ferner müsste die Zuordnung 'Fahrzeugtyp => Ausstattungsklasse' in einer Factory erfolgen, die der Konstruktor der Klasse TAuto aufruft, damit OCP nicht verletzt wird:

Delphi-Quellcode:

Type
  TAuto = Class
    FHersteller : THersteller;
    FAusstattung : TAusstattung;
  public
    constructor Create (aHersteller : THersteller);
    Property Ausstattung : TAusstattung read FAusstattung;
  end;

...
constructor TAuto.Create(aHersteller : THersteller);
begin
  FAusstattung := TAusstattungFactory.Create(aHersteller);
end;

[alda]

Zitat von Dejan Vu:

Zitat von alda:

...

• Ableitungen pro Autohersteller halte ich im allgemeinen auch nicht für sinnvoll

...

Sind sie aber. Alles andere (Enum/ Eigenschaft 'Fahrzeugtyp') bedeutet auch: Verletzung des OCP, d.h. die 'Auto-Klasse' müsste jedes mal angefasst werden, wenn eine neue Type mit spezifischen Eigenschaften hinzukommt. Hast Du dagegen pro Hersteller eine eigene Klasse, bleiben alle existierenden Klassen beim Hinzufügen einer neuen Klasse unangetastet, was den Code wesentlich robuster, d.h. stabiler macht.

Nur wenn es je Hersteller garantiert keine individuellen Eigenschaften gäbe (was hier nicht er Fall ist), muss man keine einzelne Klassen bilden.
[/DELPHI]

Da stimme ich Dir zu, allerdings sehe ich den Hersteller eher als Baustein, weniger als Herzstueck der Auto-Klasse an. Die Frage wäre welche Informationen des Herstellers benötigt werden und was damit im weiteren Verlauf ermittelt werden soll.

Zitat von Dejan Vu:

EDIT: Habe deinen Post gerade noch einmal gelesen: Du meinst, ein TAuto, aber je Hersteller unterschiedliche Ausstattungen? Das wäre eine Möglichkeit, wobei diese Architektur impliziert, das man den Hersteller wechseln kann. Weiterhin müsste man die Eigenschaft 'Ausstattung' jedes Mal casten, um auf spezifische Eigenschaften eines Herstellers zu gelangen.
Ferner müsste die Zuordnung 'Fahrzeugtyp => Ausstattungsklasse' in einer Factory erfolgen, die der Konstruktor der Klasse TAuto aufruft, damit OCP nicht verletzt wird:

Ja, oder so ähnlich. Die Frage ist, ob er in seinem Programm die Ausstattungen je Auto konfigurieren kann und welche Rolle der Hersteller spielt. Letztendlich gibt es ja auch oft den Fall, dass Hersteller A ein Auto (teilweise) anfertigt und dann Hersteller B dieses fertigstellt (z.B. individuelle Innen-Austattung) und sein Logo draufsetzt (z.B. Mercedes CITAN).
Ich sehe daher Auto, Hersteller und Ausstattung relativ abstrakt und flexibel - es muss m.E. lediglich an einer Stelle entschieden werden, wer mit wem verheiratet werden darf (z.B.: Welche Ausstattungen können für ein Auto X von Hersteller Y verbaut werden). Das könnte dann z.B. im THersteller untergebracht werden (quasi ein Ausstattungsprofil) - die komplexere Logik, welche Ausstattung eine andere aussticht oder ersetzt, sollte dann wiederum ausgelagert werden.

Aber wie immer finde ich es im allgemeinen schwierig konkrete Vorschläge zu geben, wenn wir den genauen Anwendungsfall nicht kennen. Des Weiteren glaube ich, dass es unmöglich ist den Code aufzuräumen, wenn alleine die Anpassung der Listenverarbeitungen so lange dauern sollte wie der TE vermutet hat.

Und was genau meinst Du mit "... müsste man die Eigenschaft 'Ausstattung' jedes Mal casten ...", stehe grad auf dem Schlauch?

[Dejan Vu]

Zitat von alda:

Und was genau meinst Du mit "... müsste man die Eigenschaft 'Ausstattung' jedes Mal casten ...", stehe grad auf dem Schlauch?

z.B.:

Delphi-Quellcode:

Var
  car : TCar;
...
car := myCarContainer.GetCars(herstellerMercedes).First; 

if (car.Ausstattung as TMercedesausstattung).Sitzheizung.Status=AufStufe2 then
  CheckTemperature(my.Back.Lower);

...

habe ich konkrete Autoklassen, liefert 'GetCars' direkt den konkreten Typen, d.h. ich muss nirgens casten.

Delphi-Quellcode:

Var
  mercedes : TMercedes;

...
mercedes := myCarContainer.GetCars<TMercedes>.First;

if mercedes.Ausstattung.Sitzheizung.Status=AufStufe2 then
  CheckTemperature(my.Back.Lower);

Hier sind alle Typen konkret.

Grundsätzlich bevorzuge ich die 2.Möglichkeit (ohne Typecasting auskommen), wobei das nicht notwendigerweise 'pro Autoklasse' bedeutet, aber hier zufälligerweise so ist.

[Martin W]

Danke euch sehr, vor allem das mit dem letzten Beitrag schaue ich mir näher an!

Viele Grüße!