Deine run Funktion benutzt mehr unsaubere und/oder exotische Sprachelemente als eine Version mit enum und switch.
void *sprungZiel = 0; // zustandsmerker
if (!sprungZiel) sprungZiel = && Start;
goto *sprungZiel;
sprungZiel = 0;
Wenn das deinem Kodestil entspricht, kannst du noch viel lernen, egal von wem. ;)
du noch viel lernen, egal von wem.
Genau!
Damit ich dich richtig verstehe... Kritik ja. Zeigen nein.
combie: Damit ich dich richtig verstehe... Kritik ja. Zeigen nein.
Ich habe dir gerade gezeigt wie man es einfacher verständlich und weniger exotisch formulieren kann.
Du sprachest:
Was nicht heissen soll, dass ich "einen solchen Doppelblinker" so schreiben würde...
Und ich fragte darauf hin:
Wie würdest du ihn schreiben?
Aber ist ok, wenn du nicht magst, dann lass es...
Ich habe schon verstanden, dass du vom Thema ablenken willst.
Es geht hier um goto, &&, void* versus struct und enum.
Da ist - meiner Ansicht nach - enum und switch zu bevorzugen, was ich mit meinem Beispiel versucht habe deutlich zu machen.
Wenn du anderer Ansicht bist, von mir aus.
Hallo,
goto umgewandelt in switch case, ich finde das ist besser lesbar, jetzt könnte man noch die Logikwechsel entwirren, mal sehen ob ich das auch noch hinbekomme ...
Vergleichstest ergeben beide Sketche verhalten sich gleich. Zufällig, war keine Absicht, ist der switch case Sketch um 14 Byte kleiner. Ehrlich gesagt dachte ich der wird etwas größer. Egal.
const byte taster = 2;
class Blinker
{
protected:
const byte ledPin;
const unsigned long hellPhase;
const unsigned long dunkelPhase;
unsigned long zeitMerker = 0;
typedef enum {START, HELLPHASE, DUNKELPHASE} state; // Steuerzustände
state sprungZiel = START;
public:
Blinker(const byte ledPin,const unsigned long hellPhase,const unsigned long dunkelPhase): ledPin(ledPin),hellPhase(hellPhase),dunkelPhase(dunkelPhase){}
void run(const bool blinkAnforderung)
{
switch (sprungZiel) {
case START:
if(!blinkAnforderung) break;
digitalWrite(ledPin,HIGH);
zeitMerker = millis();
sprungZiel = HELLPHASE;
break;
case HELLPHASE:
if(millis()-zeitMerkerEdit: ihr habt ja schon ohne mich weitergemacht, habt ihr die breaks in den cases absichtlich weggelassen? Der Code rammelt doch ohne break durch? Gleich einmal testen ... Edit 2: okay, wegen der jeweiligen if Abfrage wird das durchrammeln verhindert ;) Gefällt mir auch noch nicht so richtig, der Status darf erst bei Bedarf weitergeschalten werden und sollte dann auch ausgeführt werden ohne zusätzliche Hinderung, mal sehen ob mir da nochwas einfällt ...
Doc_Arduino: ihr habt ja schon ohne mich weitergemacht, habt ihr die breaks in den cases absichtlich weggelassen? Der Code rammelt doch ohne break durch? Gleich einmal testen ...
Ich habe versucht möglichst wenig zu verändern und der originale Kode fiel durch, also habe ich das so gelassen. Wenn man breaks benutzt wiirde es wahrscheinlich einen Hauch schneller, aber auch grösser werden.
Hallo,
so wenig wie möglich ändern war auch meine erste Herangehensweise. Man muss sich erstmal reindenken. Zufrieden war ich mit dem switch case noch nicht. ;) Meine neue Version ist nun auch 6 Byte größer. Egal. Ich finde es noch besser lesbar. Man sieht was wann passiert. Da man naturgemäß mit positiver Logik besser klar kommt, habe ich die positive Logik, die combie in loop mit der Tasterabfrage schon macht in switch case weitergeführt. Die Zustände namentlich machen jetzt auch das was sie aussagen. Das war mein Ziel. Bsp. der Zustand AUS bleibt aus, solange wie nicht erneut geblinkt werden soll, erst dann ändert sich der Zustand. EINSCHALTEN schaltet wirklich nur ein und übergibt in den nächsten Zustand. Danach bleibt der Zustand HELLPHASE solange erhalten wie er gültig ist, macht demnach auch hier genau das was der Name sagt usw.
Alle wieder versöhnt?
switch default könnte man hier weglassen, da alle Zustände behandelt werden, das prüft ja der Compiler.
const byte taster = 2;
class Blinker
{
protected:
const byte ledPin;
const unsigned long hellPhase;
const unsigned long dunkelPhase;
unsigned long zeitMerker = 0;
typedef enum {AUS, EINSCHALTEN, HELLPHASE, DUNKELPHASE} state; // Steuerzustände
state sprungZiel = AUS;
public:
Blinker(const byte ledPin,const unsigned long hellPhase,const unsigned long dunkelPhase): ledPin(ledPin),hellPhase(hellPhase),dunkelPhase(dunkelPhase){}
void run(const bool blinkAnforderung)
{
switch (sprungZiel) {
case AUS:
if(blinkAnforderung == true) {
sprungZiel = EINSCHALTEN;
}
break;
case EINSCHALTEN:
digitalWrite(ledPin,HIGH);
zeitMerker = millis();
sprungZiel = HELLPHASE;
break;
case HELLPHASE:
if(millis()-zeitMerker > hellPhase) {
digitalWrite(ledPin,LOW);
zeitMerker = millis();
sprungZiel = DUNKELPHASE;
}
break;
case DUNKELPHASE:
if(millis()-zeitMerker > dunkelPhase) {
sprungZiel = AUS;
}
break;
default: digitalWrite(ledPin,LOW); // sollte nie eintreten
break;
}
}
void init()
{
pinMode(ledPin,OUTPUT);
}
};
Blinker blinkGruppe[] = { // {pin,hellzeit,dunkelzeit}
{30,100, 500},
{31,500,1000},
};
void setup()
{
pinMode(taster,INPUT_PULLUP);
for(Blinker &blinker:blinkGruppe) blinker.init();
}
void loop()
{
bool blinkAnforderung = !digitalRead(taster);
for(Blinker &blinker:blinkGruppe) blinker.run(blinkAnforderung);
}
habt ihr die breaks in den cases absichtlich weggelassen?
Ich, nicht absichtlich. Eher Faulheit.
Ein break bzw. return ist bei dem Ablauf, an der Stelle, nicht nötig. (Edit: wie du ja gerade selber bemerkt hast)
Und was nicht nötig ist, lasse ich gerne weg.... Mein Fokus liegt da eher auf sprechende Bezeichner, möglichst flache Struktur, einfache Bedingungen. u.s.w.
Hallo,
sprechende Bezeichner ...
genau das macht laut meiner Meinung die neue Version. Die alte Version war schon immer einen Zustand weiter der dann wiederum künstlich aufgehalten wurde. Das musste ich entknoten. ;)
wünsche allen noch einen ruhigen sonnigen Sonntag.
PS: wenn jemand noch die Frage zu #47 (46) beantworten kann, bin ich mit allen glücklich und zufrieden
habe ich die positive Logik,... in switch case weitergeführt.
Und dir damit eine weitere Einrückungsebene eingehandelt.
Ich will damit nicht sagen, dass Blockklammer Schachtelungstiefen per se böse sind. Das sind sie nicht. Aber der Mensch kann nicht viel auf einen Blick erfassen. Bis zu einer Tiefe von 3 mag einfach so gehen, und da bist du jetzt angekommen. Mit der Klassenklammer auch schon bei 4. Ab 7 wirds kritisch, würde ich mal so sagen...
Meiner Erfahrung nach: Je flacher ein Code ist, desto flüssiger ist er von oben herab zu lesen.
Je tiefer verschachtelt, desto schwieriger gestaltet sich die Fehlersuche. Um so schlimmer, wenn noch komplizierte Bedingungen dazu kommen.
if(blinkAnforderung == true) Dafür fehlt mir allerdings etwas das Verständnis. Klar ist das richtig so.... Aber if(blinkAnforderung) würde meinem Hang zur Faulheit eher entsprechen.
Das ist hier alles noch lange nicht im schlimmen Bereich. (soweit ich das beurteilen mag)
Serenifly: Selbstverständlich. Die Zeit ist konstant weil switch/case i.d.R. als Sprungtabelle umgesetzt wird. Allerdings gehen die schnellen RJMPs (relative jump) "nur" +/- 2kB.
Doc_Arduino: Hallo,
noch einer wach. Mit 2kB meinst du den Speicherbereich in dem gesprungen wird oder darf die angelegte Sprungtabelle nicht 2kB belegten Speicher überschreiten?
siehe: RJMP Die Sprungtabelle kannst du so groß machen, wie du willst. Aber der Kompiler tut das nicht. Ab wenige Dutzend case im Switch stellt er auf eine if Artige Entscheidungsliste um. Mit den bekannten Laufzeit Nachteilen.
RJMP und Sprungtabelle haben übrigens nicht unbedingt was miteinander zu tun.
Bei Sprungtabellen liegen oftmals nur die Zieladressen im Speicher Bei RJMP ist die Zieladresse fest im Kommando einkompiliert.
Legst du allerdings RJMP Kommandos in die Tabelle, dann darf die Tabelle wirklich nicht größer sein, denn man möchte ja meist ein Ziel außerhalb der Tabelle anspringen.
Hi
Doc_Arduino: PS: wenn jemand noch die Frage zu #47 (46) beantworten kann, bin ich mit allen glücklich und zufrieden
Relative Sprünge haben nur einige Bits Platz in dem Befehl, deshalb gehen relative Sprünge auch nur x Adressen hoch oder runter. Wenn der Abstand zu groß wird, setzt der Compiler einen negierten relativen Sprung über einen absoluten Sprung. Statt einem brcc (branch relativ if carry clear) wird ein brcs +2 (branch relativ if carry set) über den jmp-Befehl gesetzt, jmp Ziel-Adresse Absolut. Wenn rjmp nicht mehr ausreicht, springt man mit negierter Logik über einen JMP, also absoluten Jump.
Habe gerade nur das Datenblatt des ATtiny45 offen, Da sind es 7 Bit, womit Er -64 ... +63 Worte springen kann. Dem nackten Chip ist Es egal, wie groß eine Tabelle ist, für Ihn sind's eh nur irgend welche Bytes, Die das Programm sinnvoll zu nutzen weiß - je nach Progger :o (der Compiler wird aber wissen, was Er Da verzapft)
MfG
Hallo,
RJMP ... okay, habe wohl übersehen. Danke.
Zum ganzen Rest muss ich nun leider sagen, du suchst irgendwie nach Ausflüchte. Anders gesagt verteidigst du deine Meinung genauso hartnäckig wie ich meine verteidige.
4 Zustände haben nichts mit Schachtlungstiefe zu tun. Das sind schlicht 4 Vergleiche auf gleicher Ebene. Es ersetzt nur deinen ersten if Vergleich vorm Label Start. Es macht nichts anderes nur das es nun ein case ist. :o Darf es demzufolge Zustandsautomaten mit mehr als 3 Vergleich nicht geben ... :o Es gibt nichts einfacheres wie 100 namentlich benannte (oder Ganzzahlen) case Vergleiche im Code zu lesen. Die if Zeitvergleiche sind hier wie da vorhanden nur jetzt in der Logik umgedreht. Keine Änderung der Tiefe. Und ob man nun == true) schreibt oder nicht, hat mit all dem nichts zu tun. Wie gesagt, die Zustände machen jetzt genau das was der Name sagt. Ist weniger verwirrend.
Kurzum, du wolltest sehen, ich hab gezeigt. Dabei belasse ich es auch. Sonst diskutieren wir noch über Syntaxe. Das mach keinen Sinn.
Zum ganzen Rest muss ich nun leider sagen, du suchst irgendwie nach Ausflüchte.
Schwachsinn!
Auch vorher schon im Thread habe ich gesagt, dass du tiefer schachteln wirst. Denn ich kann in die Zukunft schauen. Ich 2 tiefer gesagt, und du 2 tiefer gemacht.
4 Zustände haben nichts mit Schachtlungstiefe zu tun. Das sind schlicht 4 Vergleiche auf gleicher Ebene.
Du hast mich nicht verstanden.
Bei dir: Tiefe 1 Klassenblock Tiefe 2 Methodenblock Tiefe 3 Switch Block Tiefe 4 If Block
Bei mir: Tiefe 1 Klassenblock Tiefe 2 Methodenblock
Dein Code hat eine Schachtelungstiefe von 4, meiner von 2
Kurzum, du wolltest sehen, ich hab gezeigt
Meinen Dank dafür.
Anders gesagt verteidigst du deine Meinung genauso hartnäckig wie ich meine verteidige.
Ich stelle den "Goto ist immer böse" Dogmatismus an den Pranger. Mehr nicht. Und wenn du dabei bleiben willst, ist das deine Sache. Aber so wie mir scheint, hast selbst du mittlerweile geschnallt, dass diese dogmatische Sicht ein Irrweg ist. Nur ein nachplappern irriger Ansichten.
Es geht nicht um "goto ist immer böse". (Strohmann-Argument nennt man das.)
Es gibt - unbestritten - sinnvolle Anwendungen für goto. Dennoch sollte man es nur einsetzen, wenn es nötig ist, d.h. wenn eine Implementation mit anderen Konstrukten verwirrend oder super schwierig ist.
Auch Amputationen sind nicht immer böse, aber man sollte sie nur durchführen, wenn sie nötig sind.
Es gibt - unbestritten - sinnvolle Anwendungen für goto.
Und ebenso unbestritten ist es möglich damit Mist zu bauen! Der Mist hat sogar einen Namen: Spaghetticode
Und das ist eben die Argumentation die Doc_Arduino ins Feld geführt hat. Alle seine Argumente sind gültig gegen Spaghetticode. Aber nicht prinzipiell auf Goto zutreffend.
Offenbar hat er Spaghetticode und Goto unreflektiert in einen Topf geworfen.
combie: Und ebenso unbestritten ist es möglich damit Mist zu bauen! Der Mist hat sogar einen Namen: Spaghetticode
So hart hätte ich deine Implementation oben jetzt nicht bezeichnet, aber im Grunde hast du Recht.
Hallo,
muss doch nochmal antworten, auch wenn mir das langsam zu blöd wird. Wenn du jetzt noch mit dem Begriff "Spaghetticode" nach mir wirfst, dann wird es nur noch unsachlich. Deine Erklärungen sind nicht stimmig. Ist mir an den Haaren herbeigezogen. Du hast demnach keine if Blöcke? Ich sehe jede Menge davon. Auch wenn die geschweiften Klammern fehlen sind es welche. Der switch-case Code ist nicht komplizierter. Im Gegenteil aufgeräumter, einfacher nachvollziehbar. Die Jongliererei mit dem Zeiger ist auch weg. Genau darum ging es mir. Nicht mehr und nicht weniger. Man muss ja die gesamte run Funktion betrachten und nicht nur den switch Teil. Alles andere macht keinen Sinn. Wenn man hier noch default weg lässt, dann ist er auch nur unwesentlich länger. Aber gut, wir haben darüber gesprochen, es ist dein Code, wenn dir goto lieber ist, dann ist das eben dein Programmierstil. Jeder hat seinen eigenen.
Doc_Arduino: Wenn du jetzt noch mit dem Begriff "Spaghetticode" nach mir wirfst,
Mit dem Begriff hat er nach seinem Kode geworfen, nicht nach deinem (oder hast du goto's benutzt?):