Ampelprogrammierung, and innerhalb von if Anweisungen

[Detroit]

Hallo,
Ich würde gerne wissen, was in dem folgenden Quelltext falsch ist.
Bis zu der entsprechenden Zeile funktioniert auch alles einwandfrei, aber in dieser Zeile:
Shape1.Brush.Color = clRed and Shape2.Brush.Color = clYellow ist irgendwo der "Wurm drin"
Es geht um eine Ampelprogrammierung, welche ihre Farbe beim Klicken des Buttons immer um eine Ampelfarbe weiter ändert,
also von rot nach rot-gelb, von rot-gelb nach grün, von grün nach gelb und von gelb wieder nach rot, anschließend wieder von vorne.
Was ist an dem Quelltext falsch (hier der Ganze):

Code: Alles auswählen

procedure TForm1.Button1Click(Sender: TObject);
begin
  if Shape1.Brush.Color = clWhite
  then Shape1.Brush.Color := clRed
  else if Shape1.Brush.Color = clRed
  then Shape2.Brush.Color := clYellow
  else if
    Shape1.Brush.Color = clRed and Shape2.Brush.Color = clYellow
  then Shape3.Brush.Color := clLime
  else if Shape3.Brush.Color = clLime
  then Shape2.Brush.Color := clYellow
  else if Shape2.Brush.Color = clYellow
  then Shape1.Brush.Color := clRed;
 
end;
end.

Mit freundlichen Grüßen,

Nico M.

[Socke]

Willkommen im Forum!
In der Zeile

Code: Alles auswählen

if Shape1.Brush.Color = clRed and Shape2.Brush.Color = clYellow

benutzt du mehrere Operatoren: den Vergleichsoperator "=" und ein binäres Und "and" (der Wert von clRed wird mit dem aktuellen Wert von Shape2.Brush.Color kombiniert). Der Und-Operator wird zuerst ausgeführt, also steht da eigentlich:

Code: Alles auswählen

if (Shape1.Brush.Color = (clRed and Shape2.Brush.Color)) = clYellow

Abgesehen davon, dass das nicht das ist, was du willst, ergibt der Vergleichs-Operator "=" immer einen Wahrheitswert (Boolean) und den kannst du nicht mit einer Farbe (clYellow) vergleichen.

==> Klammern setzen.

[Detroit]

Das mit den Klammern habe ich auch schon auspropiert, sah dann so aus:

Code: Alles auswählen

if (Shape1.Brush.Color = clRed) and (Shape2.Brush.Color = clYellow)

Funktioniert aber auch nicht. Da ist die Fehlermeldung zwar weg, allerding springt die Ampel nur auf rot und dann von rot auf rot-gelb.
Danach geht es nicht mehr weiter.
Warum, woran liegt das?
Und warum ist das nicht das, was ich haben wollte?

Gruß.

[martin_frb]

Also ich habs nur oberflächlich betrachtest...

aber wenn du z.b Rot einschaltest, wo schaltest du gruen ab (hape3.Brush.Color = clWhite)?

Außerdem, wuerd ich den sttate anders speichern

Code: Alles auswählen

type
TAmpeState = ( asGreen, asYellow, asRed, asRedYellow)
 
var 
 state: TAmpelState;
 
if state = asRedYellow then state := asGreen else state := succ(state);
 
ODER
if state = high(TAmpelState)  then state := low(TAmpelState)  else state := succ(state);
 
 
case state of
.....

[martin_frb]

Zum Thema was falsch ist

Code: Alles auswählen

procedure TForm1.Button1Click(Sender: TObject);
begin
if Shape1.Brush.Color = clWhite
then Shape1.Brush.Color := clRed
else 
 
if Shape1.Brush.Color = clRed   // fur rot-gelb ist das trotzdem wahr.
 
then Shape2.Brush.Color := clYellow
else if
Shape1.Brush.Color = clRed and Shape2.Brush.Color = clYellow
then Shape3.Brush.Color := clLime
else if Shape3.Brush.Color = clLime
then Shape2.Brush.Color := clYellow
else if Shape2.Brush.Color = clYellow
then Shape1.Brush.Color := clRed;

[Detroit]

Das ist doch aber genau so, wie ich das auch habe?
Könntest du das ggf. so verbessern, dass es funktioniert, dann sehe ich meinen Fehler bestimmt am besten.
Ich kann dir ja mal den ganzen Quelltext einfügen, dann kannst du es dir mal schnell anschauen, wäre echt lieb.
Was mir allerdings wichtig ist, dass es mit genau diesen Befehlen gemacht werden soll, nicht mit anderen.

Gruß,

Nico M.

[u-boot]

Einwurf: Bitte Highlighter und ordentliche Formatierung nutzen. ---> Dankeschön!
Dann erledigt sich der Rest bestimmt von allein.

[Detroit]

Kannst du mir nicht vielleicht auch helfen?
Will weiter machen

[martin_frb]

Das ist doch aber genau so, wie ich das auch habe?

Ich habe einen Kommentar in deinen Code eingefügt...

Code: Alles auswählen

if Shape1.Brush.Color = clWhite
then Shape1.Brush.Color := clRed
else
 
//Du testest ob rot gesetzt is, das ist auch wahr fuer rot-gelb.
if Shape1.Brush.Color = clRed 
then Shape2.Brush.Color := clYellow
// Dann kommt ein ELSE.
else 
// Also wenn rot NICHT gesetzt ist, dann testest du
if Shape1.Brush.Color = clRed and Shape2.Brush.Color = clYellow
then Shape3.Brush.Color := clLime

Im else block kann rot nicht gesetzt sein, daher kann rot=on UND gelb = on auch nicht wahr sein

Ansonsten hatte ich ja schon ne alternative Lösung vorgeschlagen....

[Detroit]

Dann funktionierts aber immer noch nicht wenn es so aussieht:

Code: Alles auswählen

if Shape1.Brush.Color = clWhite
then Shape1.Brush.Color := clRed
else
 
//Du testest ob rot gesetzt is, das ist auch wahr fuer rot-gelb.
if Shape1.Brush.Color = clRed
then Shape2.Brush.Color := clYellow
// Dann kommt ein ELSE.
else
// Also wenn rot NICHT gesetzt ist, dann testest du
if (Shape1.Brush.Color = clRed) and (Shape2.Brush.Color = clYellow)
then Shape3.Brush.Color := clLime

hier schaltet die Ampel bis auf rot-gelb, aber von da aus immer noch nicht auf grün.

[MAC]

Wieso so kompliziert wenn es auch einfach geht ???
Globale Variable:

Code: Alles auswählen

var
  durchgang;integer;

Ein Timer, der auf Interval von 1000 gesetzt ist:
Edit: von mir aus eauch einfach ein Button , ändert nichts...

Code: Alles auswählen

//reset
Shape1.Brush.Color := clwhite;
Shape2.Brush.Color := clwhite;
Shape3.Brush.Color := clwhite;
 
//farbe
case durchgang of
0:begin
  Shape1.Brush.Color := clred;
  end;
1:begin
  Shape1.Brush.Color := clred;
  Shape2.Brush.Color := clyellow;
  end;
2:begin
  Shape3.Brush.Color := clgreen;
  end;
3:begin
  Shape3.Brush.Color := clyellow;
  end;
end;
inc(druchgang); // inc = erhöht durchgang um 1, also das selbe wie durchgang := durchgang +1;
if durchgang > 3 then durchgang := 0;

Das ist viel besser, weil hier nicht die Farbe der Ampel abgefragt wird sondern eine andere Variable dafür genutzt wird...
Mal davor abesehen, das es 40% mehr Rechenzeit, also 0.01 Sekunden mehr brauch, da ich zur Übersicht das RESET nicht in die Case abfrage getan habe...

[Detroit]

Das verstehe ich als unerfahrener Lazarus Nutzer aber nocht nicht, habe mich allerdings an einen weiteren Versuch gewagt, vielleicht könnt ihr den ja sonst so verbessern, dass er klappt, die Ampel schaltet wieder nur bis rot und gelb zusammen leuchten:

Code: Alles auswählen

procedure TForm1.Button1Click(Sender: TObject);
begin
If (Shape1.Visible = false) and (Shape2.visible = true) and (Shape3.visible = false)
then
begin
Shape1.Visible := true;
Shape2.Visible := false;
Shape3.Visible := false;
end
Else
If (Shape1.visible = true) and (Shape2.visible = false) and (Shape3.visible = false)
then
begin
Shape1.Visible := true;
Shape2.Visible := true;
Shape3.Visible := false;
end
else
If (Shape1.visible = true) and (Shape2.visible = true) and (Shape1.visible = false)
then
begin
Shape1.Visible := false;
Shape2.Visible := false;
Shape3.Visible := true;
End
else
if (Shape1.Visible = false) and (Shape2.Visible = false) and (Shape3.Visible = true)
then
begin
Shape1.Visible := false;
Shape2.Visible := true;
Shape3.Visible := false;
    end
end;
end.

Das ist sicherlich ein sehr langer Lösungsweg, aber man muss den doch auch so schreiben können, halt nur, dass er dann auch zum gewünschten Resultat führt
Also was ist falsch?

Danke und Gruß !

[MAC]

Einwurf: Bitte Highlighter und ordentliche Formatierung nutzen. ---> Dankeschön!
Dann erledigt sich der Rest bestimmt von allein.

Bitte Beachten...
Das ließt sich sonst sehr schwer...
Einfach den Text makieren, auf die Highlighter-Combobox klicken und Freepascal/Lazarus auswählen, danke.

Was falsch ist, Eigentlich nichts, aber bei der Dritten If- abfrage steht das hier

Code: Alles auswählen

If (Shape1.visible = true) and (Shape2.visible = true) and (Shape1.visible { HIER STEHT Shape1 statt Shape3} = false)

richtig wäre es

Code: Alles auswählen

If (Shape1.visible = true) and (Shape2.visible = true) and (Shape3.visible = false)

Also nicht ein einfacher fehler
Natürlich müssen alle Shapes von Anfang an schon in einer der 4 gültigen Positionen sein...

Jetzt erkläre ich mal den Code oben...
Das Abfragen ob bestimmte Shapes sichtbar sind oder nicht ist immer kompliziert, bzw manchmal unmöglich, manchmal aber auch nur umständlich, da wenn man weis das Das Gelbe lciht leutet nicht automatisch weis ob jetzt rot oder grün kommt. Dshalb muss zusätlich noch überprüft werden ob rot leuchtet...

Deshalb nimmst du einfach eine Variable (sollte kein Problem sein) und welche Farbe erscheint liegt jetzt nicht daran welche Farbe vorher geherscht hat, sondern welchen wert diese variable hat...

ist diese Variable auf 0 steht die Ampel auf rot
ist diese Variable 1 steht sie auf Rot-Gelb
ist sie 2 seht die Ampel auf grün
ist sie 3 steht sie auf gelb
ist sie größer als 3 wird sie wieder auf 0 gesetzt, damit es von vorne losgeht...

Dieses Case ist im grunde nur eine Schöne schreibart für mehrere If then else if then else ...

Am anfang wird alles auf weis gesetzt, ist ja kein Problem denn wir brachen den alten Farbstand ja nicht, da wir die variable durchgang haben...
Durchgang wird um eins erhöht, und wenn es größer 3 ist wird es wieder auf 0 gesetzt...

[martin_frb]

Also ich empfehle auch dringend eine extra variable zu benutzen, um den aktuellen zustand ("state") zu speichern.
Es gibt vier Zustände, als z.b Zähler von 1 bis 4.

Aber um auf deine IF Variante zurückzukommen.
Sie ist zwar länger, schlechter zu lesen, und fehleranfälliger, ABER zum lernen trotzdem ein schritt den Du machen kannst.

als erstes:
verwende anständige NAMEN: Shape1, könnte man ja auch "RotesLicht" nennen...
(Im object inspector unter "Name" umbenenen.

zweitens, nicht zwingend, aber besser sich dran zu gewoehnen

Code: Alles auswählen

If (Shape1.Visible = false) and (Shape2.visible = true) and (Shape3.visible = false)

Code: Alles auswählen

If (not Shape1.Visible) and (Shape2.visible) and (not Shape3.visible)

Mit anständigen Namen würdest du folgende Zeile sehen:
If (Shape1.visible = true) and (Shape2.visible = true) and (Shape1.visible = false)

die hat 2mal Shape1 => soll soll hinten Shape3 sein.

[Detroit]

Danke erstmal an alle
Das mit dem true und false funktioniert jetzt einwandfrei (:
Bei dem Case Modell habe ich es leider nicht geschafft.
Mein Quelltext sieht jetzt wiefolgt aus:

Code: Alles auswählen

procedure TForm1.Button1Click(Sender: TObject);
var x : integer;
begin
case x of
0:begin
  Shape1.Brush.Color := clred;
  end;
1:begin
  Shape1.Brush.Color := clred;
  Shape2.Brush.Color := clyellow;
  end;
2:begin
  Shape3.Brush.Color := clgreen;
  end;
3:begin
  Shape2.Brush.Color := clyellow;
  end;
end;
inc(x); // inc = erhöht durchgang um 1, also das selbe wie durchgang := durchgang +1;
if x > 3 then x := 0;
end;
end.

Edit: bei dem 4. Fall müsste es übrigens Shape2 sein, oder habe ich einen Denkfehler?