if-Anweisungen als Block

[__Son´s B. (bersison)]

Hallo.
Vesuche Prog (C) zu optimieren, daher die Frage ob folgende Routine 
"legetim" ist;
______________________________________
SprungZiel = 0;
if (LadSpg_ON) SprungZiel=2; // wenn Ladespg vorhanden
if ((PORTA & (1<<PA4)) == 0) SprungZiel=2; // wenn PORT_BATT_OUT low
if ((PINB & (1<<PB0)) != 0) SprungZiel=1; // wenn Eing. SPERRE_IN high

switch(SprungZiel)
{
case 1:  // SPERRE_IN high
...
break;

case 2: // PORT_BATT_OUT high
...
break;

default: // weitere Messung, Auswertung, Steuerung
...
break;
}
_________________________________________

Verständnisfrage:
Das Prog. durchläuft zwingend(?!) alle 3 if-Aweisungen nacheinander.
Sobald einer der 3 if-Anweisungen "wahr" ist, verändert sich die 
Variable "SprungZiel" auf 1 oder 2.

[Stefan F. (Gast)]

Ja, ist ok. Hat niemand verboten und tut das, was du beschrieben hast.

[Einer K. (Gast)]

__Son´s B. schrieb:
> ob folgende Routine
> "legetim" ist;

Welche Probleme hättest du denn gerne?

[Frank L. (Firma: Flk Consulting UG) (flk)]

Hallo,
die Frage ist mit JaEin zu beantworten.

Wenn eine der beiden ersten Anweisungen SprungZiel = 2 gesetzt haben, 
darf dann trotzdem noch SprungZiel = 1 angesprungen werden?

Die beiden ersten Anweisung kannst Du zusätzlich zu einem if 
zusammenfassen und die beiden Entscheidungen oder verknüpfen.

if (LadSpg_ON || (PORTA & (1<<PA4) == 0)) SprungZiel=2;

Gruß
Frank

[Michael A. (micha54)]

Hallo,

also so ganz toll finde ich diesen Programmierstil nicht:

Wenn im oberen Teil mehrere Bedingungen true sind, wird SprungZiel 
mehrfach zugewiesen. Nur das letzte wird dann tatsächlich ausgeführt.

Intuitiv würde ich in der if then - Kette erwarten, dass die wichtigsten 
Bedingungen zuerst notiert sind.

Ungewöhnlicher Programmierstil und eine wenig intuitive Vorgehensweise 
können zu Fehlern führen, falls sich mal ein Kollege dran betätigen 
muss.

Lösung: man könnte oben if then else if then else abprüfen, dann wäre 
Sprungziel wieder das intuitive Ergebnis.

Gruß,
Michael

[Falk B. (falk)]

@__Son´s Bersi__ (bersison)

>Vesuche Prog (C) zu optimieren,

Weißt du überhaupt, wie man das richtig macht? Und warum?

https://www.mikrocontroller.net/articles/AVR-GCC-Codeoptimierung#Prinzipien_der_Optimierung

>daher die Frage ob folgende Routine
>"legetim" ist;

Sie ist OK, optimiert aber nichts wirklich. Dein Programm wird weder 
kürzer, noch schneller, noch besser lesbar. Wenn man schon viele 
verschiedene Bedingungen prüfen muss, ist eine if/else Kette sinnvoller. 
Deine Version ist doppelt gemoppelt.

if (LadSpg_ON || !((PORTA & (1<<PA4)) ) {
  // mach was
} else if ((PINB & (1<<PB0)) ) {
  // mach was anderes
}

>Das Prog. durchläuft zwingend(?!) alle 3 if-Aweisungen nacheinander.

Sicher, es gibt ja keinen else-Zweig.

[HildeK (Gast)]

__Son´s B. schrieb:
> SprungZiel = 0;
> if (LadSpg_ON) SprungZiel=2; // wenn Ladespg vorhanden
> if ((PORTA & (1<<PA4)) == 0) SprungZiel=2; // wenn PORT_BATT_OUT low
> if ((PINB & (1<<PB0)) != 0) SprungZiel=1; // wenn Eing. SPERRE_IN high

Frank Link hat es schon angedeutet.
Wenn LadSpg_ON wahr ist, dann kann ja trotzdem PB0 != 0 sein. SprungZiel 
wird dann zunächst auf 2 gesetzt und dann aber auf 1. Ist das dein 
Wunsch?

Oft ist es sinnvoll, in den eigentlichen case-Blöcken den 
Switchparameter (SprungZiel) neu zu setzen für den nächsten State.
Zugegeben, ich habe nicht versucht zu überlegen (es fehlen auch Infos 
dazu), was diese Abläufe tun sollen.

[Einer K. (Gast)]

Falk B. schrieb:
> ist eine if/else Kette sinnvoller

Ich mag keine tief geschachtelten if Dinger, mit komplizierten 
Bedingungen.

Persönliche Ansicht: Je naiver, desto besser.

----
Wenn das ein endlicher Automat sein soll, dann sollte man die Schritte 
klar benennen, und die Weiterschaltbedingungen, dahin tun, wo sie 
hingehören, in die Case Bereiche..

[__Son´s B. (bersison)]

HildeK schrieb:
> Frank Link hat es schon angedeutet.
> Wenn LadSpg_ON wahr ist, dann kann ja trotzdem PB0 != 0 sein. SprungZiel
> wird dann zunächst auf 2 gesetzt und dann aber auf 1. Ist das dein
> Wunsch?

Ja!
Die Abfolge ist von oben herab umgekehrt Priorisiert.
D.h., dass die unterste if-Abfrage die Wichtigste ist (Notaus-Meldung).

Auf "else-Verschachtelungen" wollte ich aufgrund einfacherer/schnellerer 
Lesbarkeit (einfache Listenansicht) verzichten.

[Thomas E. (thomase)]

__Son´s B. schrieb:
> Auf "else-Verschachtelungen" wollte ich aufgrund einfacherer/schnellerer
> Lesbarkeit (einfache Listenansicht) verzichten.

Else bedeutet nicht zwangsläufig, daß man etwas verschachtelt und es 
schwerer lesbar wird. Dein Verzicht bedeutet aber, daß man den Sinn 
nicht erkennt.

if (LadSpg_ON) SprungZiel=2; // wenn Ladespg vorhanden
if ((PORTA & (1<<PA4)) == 0) SprungZiel=2; // wenn PORT_BATT_OUT low
if ((PINB & (1<<PB0)) != 0) SprungZiel=1; // wenn Eing. SPERRE_IN high

Wenn du priosieren willst dann mach das so

if ((PINB & (1<<PB0)) != 0) SprungZiel=1; // wenn Eing. SPERRE_IN high
else if ((PORTA & (1<<PA4)) == 0) SprungZiel=2; // wenn PORT_BATT_OUT low
else if (LadSpg_ON) SprungZiel=2; // wenn Ladespg vorhanden

Damit erreichst du das gleiche und jedem ist klar, was das soll. Und 
schneller geht es auch noch. Die Zusammenfassung der beiden letzten 
Bedingungen habe ich mir jetzt mal erspart. Die fasst der Compiler auch 
von sich aus zusammen.

[Alexander B. (Firma: brickwedde.dev) (alexbrickwedde)]

Und jetzt noch den Reset von SprungZiel in den "finalen" else Zweig:

if ((PINB & (1<<PB0)) != 0) SprungZiel=1; // wenn Eing. SPERRE_IN high
else if ((PORTA & (1<<PA4)) == 0) SprungZiel=2; // wenn PORT_BATT_OUT low
else if (LadSpg_ON) SprungZiel=2; // wenn Ladespg vorhanden
else SprungZiel=0;

[Eric B. (beric)]

__Son´s B. schrieb:
> Hallo.
> Vesuche Prog (C) zu optimieren, daher die Frage ob folgende Routine
> "legetim" ist;

Es kompiliert, es stürzt zur Laufzeit nicht ab, also: Ja.

> Verständnisfrage:
> Das Prog. durchläuft zwingend(?!) alle 3 if-Aweisungen nacheinander.
> Sobald einer der 3 if-Anweisungen "wahr" ist, verändert sich die
> Variable "SprungZiel" auf 1 oder 2.

Und wo ist die Frage?

Optimaler wäre eher:

if ((PINB & (1<<PB0)) != 0)        // wenn Eing. SPERRE_IN high
{
   ... Code von case 1...
}
else if ((LadSpg_ON)               // wenn Ladespg vorhanden
     || ((PORTA & (1<<PA4)) == 0)) // wenn PORT_BATT_OUT low
{
  ...Code von case 2...
}
else 
{
  ...Code von Default... // weitere Messung, Auswertung, Steuerung
}

[Thomas E. (thomase)]

Genau.

Und den Switch-Kram erledigt man auch gleich an Ort und Stelle:

if (PINB & (1<<PB0))
{
//Das case 1-Zeugs
}
else if (!((PORTA & (1<<PA4)) || LadSpg_ON))
{
//Das case 2-Zeugs
}
else
{
//Das default-Zeugs
}

Edit: Uups, da war einer schneller

[Peter D. (peda)]

Arduino F. schrieb:
> Ich mag keine tief geschachtelten if Dinger, mit komplizierten
> Bedingungen.

Ich auch nicht.
Ich schreibe es daher so:

if ((PINB & (1<<PB0)) != 0)
  return 1;
if (LadSpg_ON)
  return 2;
if ((PORTA & (1<<PA4)) == 0)
  return 2;
return 0;

[__Son´s B. (bersison)]

Erst einmal DANK an alle inspireierenden Anregungen!

Ich mag die Listenarbeiten lieber als Versachachtelungen, daher finde 
ich if-Blöcke und anschliessend switch-Blöcke übersichtlicher.
GESCHMACKSSACHE!

Werde aber zukünftig if/else-Formen wahren;

SprungZiel=0
if ((PINB & (1<<PB0)) != 0) SprungZiel=1; // wenn Eing. SPERRE_IN high
else if ((PORTA & (1<<PA4)) == 0) SprungZiel=2; // wenn PORT_BATT_OUT 
low
else if (LadSpg_ON) SprungZiel=2; // wenn Ladespg vorhanden

else SprungZiel=0;  // überflüssig, da am Schleifenanfang "0" gesetzt 
wird

[Eric B. (beric)]

__Son´s B. schrieb:
> Werde aber zukünftig if/else-Formen wahren;

...und für Variabelen sinnvolle Namen benutzen, z.B. "Aktion" statt 
"Sprungziel", und bedeutungsvolle Namen definieren für Konstanten, oder 
sogar eine enum dafür!

typedef enum {
  DO_NOTHING = 0,
  BAKE_CAKE,
  SWITCH_OFF_OVEN,
  WAIT
} Action_e;

Action_e action = DO_NOTHING;

#define SPERRE_IN_HIGH ((PINB & (1<<PB0))
#define BATT_OUT_LOW   (!(PORTA & (1<<PA4)))

...
if (SPERRE_IN_HIGH)
  action = BAKE_CAKE;
else if(BATT_OUT_LOW || LadSpg_ON)
  action = SWITCH_OFF_OVEN;
else
  action = DO_NOTHING;

Switch(action) {
  case BAKE_CAKE:
  ...
  break;

...usw...

[Ralf G. (ralg)]

__Son´s B. schrieb:
> Erst einmal DANK an alle inspireierenden Anregungen!

Moooment! ;-)
Wenn ich mich nicht irre, stimmen die vorgeschlagenen Lösungen nicht mit 
der ursprünglichen Anforderung überein:

__Son´s B. schrieb:
> D.h., dass die unterste if-Abfrage die Wichtigste ist (Notaus-Meldung).

Peter D. schrieb:
> if ((PINB & (1<<PB0)) != 0)
>   return 1;
Eric B. schrieb:
> Optimaler wäre eher:if ((PINB & (1<<PB0)) != 0)        // wenn Eing.
> SPERRE_IN high
> {
>    ... Code von case 1...

Sobald die erste Bedingung erfüllt ist, wird auch der dazugehörige Code 
ausgeführt. Das 'Notaus' ist irgendwo gaaaanz weit hinten im 'else' 
versteckt. Also für alle Bedingungen keine Priorisierung wie gefordert.

[A. S. (Gast)]

__Son´s B. schrieb:
> Ich mag die Listenarbeiten lieber als Versachachtelungen

Dann mal so am Rande die Frage, ob Du einen guten Editor verwendest.

[Falk B. (falk)]

@__Son´s Bersi__ (bersison)

>Ich mag die Listenarbeiten lieber als Versachachtelungen, daher finde
>ich if-Blöcke und anschliessend switch-Blöcke übersichtlicher.
>GESCHMACKSSACHE!

Naja, wenn du das in (semi)professionellem Umfeld anwenden willst, ist 
es das nicht, denn andere Leute müssen damit klar kommen. Und das wird 
den Wenigsten gefallen.

Außerdem geht das schon leicht in Richtung Josef'scher "Logik", Mr. Bo8

[Eric B. (beric)]

Falk B. schrieb:
> Außerdem geht das schon leicht in Richtung Josef'scher "Logik", Mr. Bo8

ROFL! YMMD! :-D