Effektiver machen

[FabianIBerlin]

hi, ich habe bei meinem Schulprojekt in einem Thread eine aktualisierung von mehrern Shapes und labels vrgenommen.
Jetz sind das aber ellen von Quelltext für die Selbe Sache und ich wollt fragen ob mir jemand sagen kann wie ich das mit ner schelife oder so schlanker machen könnte

(hier ein auschnit der aktualisiereung)

Delphi-Quellcode:

//Nonnendamm Visualisierung
  LeitrechnerForm.NdBetriebL.Caption := KreuzungInfos[1,5];
  LeitrechnerForm.NdStatusL.Caption := KreuzungInfos[1,6];

 //Nonnendamm Hauptstraße
 if KreuzungInfos[1,3] = 'off' then
    begin
      LeitrechnerForm.HsNdRShape.Brush.Color := clBtnFace;
      LeitrechnerForm.HsNdGrShape.Brush.Color := clBtnFace;
      LeitrechnerForm.HsNdGShape.Brush.Color := clBtnFace;
    end;
  if KreuzungInfos[1,3] = 'red' then
    LeitrechnerForm.HsNdRShape.Brush.Color := clRed;
  if KreuzungInfos[1,3] = 'yel' then
    LeitrechnerForm.HsNdGShape.Brush.Color := clYellow;
  if KreuzungInfos[1,3] = 'gre' then
    LeitrechnerForm.HsNdGrShape.Brush.Color := clGreen;
  if KreuzungInfos[1,3] = 'rye' then
    begin
      LeitrechnerForm.HsNdRShape.Brush.Color := clred;
      LeitrechnerForm.HsNdGrShape.Brush.Color := clYellow;
    end;

  //Nonnendamm Nebenstrassse
  if KreuzungInfos[1,4] = 'off' then
    begin
      LeitrechnerForm.NsNdRShape.Brush.Color := clBtnFace;
      LeitrechnerForm.NsNdGrShape.Brush.Color := clBtnFace;
      LeitrechnerForm.NsNdGShape.Brush.Color := clBtnFace;
    end;
  if KreuzungInfos[1,4] = 'red' then
    LeitrechnerForm.NsNdRShape.Brush.Color := clred;
  if KreuzungInfos[1,4] = 'yel' then
    LeitrechnerForm.NsNdGShape.Brush.Color := clYellow;
  if KreuzungInfos[1,4] = 'gre' then
    LeitrechnerForm.NsNdGrShape.Brush.Color := clGreen;
  if KreuzungInfos[1,4] = 'rye' then
    begin
      LeitrechnerForm.NsNdRShape.Brush.Color := clred;
      LeitrechnerForm.NsNdGrShape.Brush.Color := clYellow;
    end;

//Gartenfelder Visualisierung
  LeitrechnerForm.GFBetriebL.Caption := KreuzungInfos[2,5];
  LeitrechnerForm.GFStatusL.Caption := KreuzungInfos[2,6];

  //Gartenfelder Hauptstraße
  if KreuzungInfos[2,3] = 'off' then
    begin
      LeitrechnerForm.HsGfRShape.Brush.Color := clBtnFace;
      LeitrechnerForm.HsGfGrShape.Brush.Color := clBtnFace;
      LeitrechnerForm.HsGfGShape.Brush.Color := clBtnFace;
    end;
  if KreuzungInfos[2,3] = 'red' then
    LeitrechnerForm.HsGfRShape.Brush.Color := clRed;
  if KreuzungInfos[2,3] = 'yel' then
    LeitrechnerForm.HsGfGShape.Brush.Color := clYellow;
  if KreuzungInfos[2,3] = 'gre' then
    LeitrechnerForm.HsGfGrShape.Brush.Color := clGreen;
  if KreuzungInfos[2,3] = 'rye' then
    begin
      LeitrechnerForm.HsGfRShape.Brush.Color := clred;
      LeitrechnerForm.HsGfGrShape.Brush.Color := clYellow;
    end;

  //Gartenfelder Nebenstrassse
  if KreuzungInfos[2,4] = 'off' then
    begin
      LeitrechnerForm.NsGfRShape.Brush.Color := clBtnFace;
      LeitrechnerForm.NsGfGrShape.Brush.Color := clBtnFace;
      LeitrechnerForm.NsGfGShape.Brush.Color := clBtnFace;
    end;
  if KreuzungInfos[2,4] = 'red' then
    LeitrechnerForm.NsGfRShape.Brush.Color := clred;
  if KreuzungInfos[2,4] = 'yel' then
    LeitrechnerForm.NsGfGShape.Brush.Color := clYellow;
  if KreuzungInfos[2,4] = 'gre' then
    LeitrechnerForm.NsGfGrShape.Brush.Color := clGreen;
  if KreuzungInfos[2,4] = 'rye' then
    begin
      LeitrechnerForm.NsGfRShape.Brush.Color := clred;
      LeitrechnerForm.NsGfGrShape.Brush.Color := clYellow;
    end;

(so geht das noch 4 mal)

[freak4fun]

Hallo :hi:,
hast du die Möglichkeit aus denmArray KreuzungInfos[x,y] das ein Array of String zu sein scheint ein Array of Color zu machen, dann könntest du die Farbe direkt zuweisen.

Statt:

Delphi-Quellcode:

if KreuzungInfos[1,3] = 'red' then 
    LeitrechnerForm.HsNdRShape.Brush.Color := clRed;

So:

Delphi-Quellcode:

LeitrechnerForm.HsNdRShape.Brush.Color := KreuzungInfos[1,3];

Wäre jetzt so meine erste Idee. Oder schau dir mal Case ... of ... end; an. Das vereinfacht deine If-Bedingungen. ;)

MfG
freak

[Sharky]

Hai Fabian,

am effektivsten wäre es sicher sich gedanken über das gesamte Model zu machen ;-)
Da die Shapes ja alle einen anderen Namen haben ist da mit einer Schleife nicht viel zu holen. Du müsstest Dir also überlegen ob Du es so umstellen kannst damit Du auf die Shapes über einen Index zugreifen kannst.

BTW: Ist da nicht ein Fehler im Code? Wenn KreuzungInfos[1,3] = 'red' ist wird das zugehörige Shape rot. Aber wenn KreuzungInfos[1,3] danach zum beispiel 'yel' ist wird das erste Shape doch nicht in der Farbe geändert? Oder übersehe ich das eben im Code?

Ausserdem solltest Du versuchen für des Status (off,red,yel...) keine Strings zu verwenden sondern diese lieber in einem Byte zu speichern.

[ichbins]

klar geht das.


aber ich tus mir jetzt nicht an, den ganzen Code abzuschreiben.

wir sind hier ja kein hausaufgaben.de.

Nur ein paar Gedankenanstöße:

du speicherst alle Objekte in einem solchen Array.

Delphi-Quellcode:

const
objs:array[1..4,1..3] of object=((HsNdRShape,HsNdGrShape,HsNdGSShape),(nsNdRShape,nsNdGrShape,nsNdGSShape),(HsNdRShape,HsNdGrShape,HsNdGSShape),(HsNdRShape,HsNdGrShape,HsNdGSShape)...

Dann kannst du in einer Schleife die Widerholungen durchgehen.

[Sharky]

Zitat:

Zitat von ichbins

... wir sind hier ja kein hausaufgaben.de. ...

Unterlasse bitte solche Unterstellungen! :warn:
Nur weil es ein Schulprojekt ist bedeutet es nicht das er hier keine Hilfe bekommt.

[FabianIBerlin]

Zitat:

Zitat von Sharky

Hai Fabian,
BTW: Ist da nicht ein Fehler im Code? Wenn KreuzungInfos[1,3] = 'red' ist wird das zugehörige Shape rot. Aber wenn KreuzungInfos[1,3] danach zum beispiel 'yel' ist wird das erste Shape doch nicht in der Farbe geändert? Oder übersehe ich das eben im Code?

Ah stimmt du hast recht, habe das nach einer änderung wieder rückgängig gemacht...

Zitat:

Zitat von ichbins

wir sind hier ja kein hausaufgaben.de.

Es ist nicht wirklich eine Hausaufgabe ich wollte es nur machen um meinen Quellcode zu verbessern.

Aber thx an alle

[Robert Marquardt]

Sharky, ichbins hat durchaus recht. Vollstaendig geloeste Hausaufgaben sollte es hier nicht geben.
Es ist wirklich besser Denkanstoesse zu geben.

Um jetzt nochmals die Vorschlaege zu beschreiben:
Das Basisproblem kommt vom unguenstigen Typ der Elemente von KreuzungInfos.
Ein String ist kein ordinaler Typ und daher kann man ihn nicht als Arrayindex benutzen.
Man sollte also ein Enum fuer die Zustaende nehmen, da man damit dan ein Hilfsarray mit Farbwerten indizieren kann.
Direkt Farben zu nehmen ist nicht gut fuer das Design des Programms.

[Sharky]

Zitat:

Zitat von Robert Marquardt

Sharky, ichbins hat durchaus recht. Vollstaendig geloeste Hausaufgaben sollte es hier nicht geben. ...

Es ist zwar OT. Aber Fabian hat an keiner Stelle geschrieben das er einen fertigen Code möchte. Darum spach ich ja auch von einer "Unterstellung" ;-)