Hallo, ich brauche mal einen Tip:
Ich habe eine switch-case Schleife im Main. In jedem Case ist sehr viel
Code untergebracht. Gibt es jetzt eine Möglichkeit, den irgendwie
zusammenzufassen?
1
voidmain(void)
2
{
3
...
4
switch(x)
5
{
6
case0:
7
// hier liegt viel Code
8
case1:
9
// hier liegt viel Code
10
case2:
11
// hier liegt viel Code
12
...
13
}
Ich weiß, ich kann eine neue *.h und *.c Datei anlegen, und dort
Funktionen für jeden case hinterlegen, aber da habe ich keine
Möglichkeit, auf die Varaiblen zuzugreifen, die ich sonst in der case
zur Verfügung habe. (lokale von main und globale von main.c)
Also ich bräuchte sowas wie #define, aber eben mit viel code..
Jemand eine Idee?
Thx
Entweder Code so umschreiben, dass die Variablen auch in Funktionen
zugänglich sind. Im switch dann die Funktionen zum Abarbeiten des case
aufrufen.
Oder einen Editor mit Folding benutzen. Dann kann na die grossen
Codeteile bei der Bearbeitung wegklappen und sich auf den wichtigen
Codeausschnitt konzentrieren.
Oder mit #define arbeiten und \ am Zeilenende benutzen, um das Makro in
der nächsten Zeile fortzusetzen.
Du kannst den Code in den einzelnen Cases in Funktionen packen, die die
benötigten Variablen als Argumente entgegen nehmen.
Damit keine Effizienzverluste entstehen: Die Funktionen definierst du
als static, entweder in der gleichen Quellcodedatei, in der auch main
steht, oder in einer zweiten Datei, die von da includet wird. Dann wird
der Compiler diese Funktionen inlinen, was im gleichen Code wie bei
deiner jetzigen, unübersichtlichen Variante resultiern sollte.
>als Argumente entgegen nehmen
Das sind zu viele. Ich hab jetzt alle in eine globale Struktur
geschmissen...
>in der gleichen Quellcodedatei..
dann ist die unübersichtlichkeit nur weiter oben...
>zweiten Datei, die von da includet wird..
So habe ich das jetzt angefangen. Eine c/h Datei mit je einer Funktion
für einen case-Fall..
Das geht, ist eine sehr schlechte Idee. Kein Editor der Welt kann kann
dann für dich Variablen umbenennen oder alle Stellen finden, an denen
eine Variable verwendest.
Noch schlimmer wird es, wenn du vergisst irgendwo eine Klammer zu
schließen, dann kommt der Syntaxfehler womöglich noch im falschen File.
Die beste Empfehlung die man dir geben kann, ist wahrscheinlich das
Programm so umzustukturieren, dass man an diesen Stellen Funktionen
aufrufen kann. Die Tatsache, dass viele Parameter übergeben werden
müssen, deutet eher auf ein schlechtes Design hin.
Das Zusammenfassen zu einer Struktur ist sicher eine gute Idee, warum
dann nicht mehrere in denen die Variable, die irgendwie logisch
zusammengehören, zusammengefasst werden.
Zur Not gibt es auch globale Variablen, diese müssen nicht übergeben
werden.
>dass man an diesen Stellen Funktionen
So hab ichs jetzt auch gemacht..
>dass viele Parameter übergeben
Naja, etwa 5 Varaiblen sind bisher. Aber viele Konstanten ;-(
>Struktur ist sicher eine gute Idee
Ich hab alle diese Variablen in einer globalen Struktur..
Matthias Lipinsky wrote:
>>dass man an diesen Stellen Funktionen>> So hab ichs jetzt auch gemacht..>>>>dass viele Parameter übergeben>> Naja, etwa 5 Varaiblen sind bisher. Aber viele Konstanten ;-(
Ohne jetzt dein Programm zu kennen, könnte es sein, dass deine
Funktionen dann ganz einfach zuviel machen, wenn du sie mit einer
Unmenge an Konstanten steuern musst.
>>Struktur ist sicher eine gute Idee>> Ich hab alle diese Variablen in einer globalen Struktur..
Auch das deutet auf ein Problem hin. Variablen werden dann zu einer
Struktur zusammengefasst, wenn diese Variablen thematisch
zusammenghören. Scope, also ob global oder funktionslokal, ist aber kein
derartiger Themenkreis. Themenkreis weäre zb. Datum oder Name oder
Adresse oder sowas in der Richtung.
>nmenge an Konstanten steuern musst.
Damit meine ich eher Texte im Flash:
uint8_t[] TXT_MENU1 PROGMEM = "Hallo Welt";
...
Diese verwende ich in allen case-Fällen.
>wenn diese Variablen thematisch zusammenghören.
Das tun sie. Es sind jetzt zwei structs. Eine für die Schrittkette
(Zustand, Zeit,..), und eine zweite für das, was in der/den cases
gemacht werden soll: Menu bauen und auswerten.
Abends kann ich mal einen case posten.
Matthias Lipinsky wrote:
> Abends kann ich mal einen case posten.
Wird wohl das Vernünftigste sein. So ins Blaue hinein ist es immer
schwer irgendwelche Anregungen zu geben. Dazu gibt es einfach zuviele
Möglichkeiten.
Matthias Lipinsky wrote:
>>Wird wohl das Vernünftigste sein..>> Im Anhang ist der Auszug der Funktion main_menu, nachdem ich das> ausgelagert habe..
Und? Was gefällt dir daran jetzt nicht? Da sind doch gar keine
riesengroßen Parameterlisten.
Darf ich trotzdem ein paar Vorschläge machen? (... die jetzt mit deiner
unmittelbaren Fargestellung so nichts zu tun haben)
Mir ist in deinem Code Schnipsel aufgefallen, dass hier immer wieder die
Sequenz
1
strcpy_P(&au8TempStr[0],AU8TXT_MAIN1);
2
lcd4x20_show(/* u8Line */0b0001,
3
/* u8Col */1,
4
/* pu8Data */&au8TempStr[0],
5
/* u8Len */strlen_P(AU8TXT_MAIN1));
vorkommt.
* Gewöhn dir das & ...[0] wieder ab. Das bringt nichts an Klarheit
und ist nur unnötiger Tippaufwand. Der Name eines Arrays alleine
fungiert aqls Zeiger auf das erste Array Element
* Die tatsächliche Länge eines Strings s in eine Funktion mitzugeben ist
meistens Unfug. Vor allen Dingen, wenn du die Stringlänge sowieso
mittels strlen bestimmst. So was sollte dir immer eine Warnung sein,
dass dieser Parameter eigentlich unnötig ist. Die Funktion kann sich die
Länge selbst bestimmen, wenn sie sie benötigt.
* Kopiere Strings niemals um, wenn es nicht unbedingt sein muss!
Das Problem dabei: Wieviel Speicher reservierst du denn für die Kopie?
Wenn du es richtig machen willst, musst du den Speicher dynamisch
allokieren, und das wollen wir auf einem µC eigentlich vermeiden wenn es
geht.
* Nichts gegen Kommentare. Aber streu deine Kommentare nicht mitten in
den Code rein. Das hindert den Lesefluss extrem.
Wo liegt in deinem Code-Schnipsel das Problem? Du warst zu faul, dir
eine Funktion zu machen, die einen String aus dem Flash ausgeben kann.
So wie du ein lcd4*20_show hast, hättest du dir noch ein lcd4*20_show_P
machen sollen. Die Funktion kriegt: Zeile, Spalte, Zeiger auf String
(und keine Längenangabe! Das solltest du auch bei deiner lcd4*20_show
noch korrigieren)
Diese Funktion holt sich den String direkt aus dem Flash, wobei sie der
üblichen C-Konvention folgt, dass der String bei einem '\0' Zeichen
endet (was anderes macht strcpy_P auch nicht). Aber: Sie benötigt keinen
Zwischenspeicher und kann daher auch keinen Zwischenspeicher überlaufen!
Auch benötigt sie die tatsächliche String Länge nicht, ist daher
schneller, als wenn vorher mittels strlen_P die Länge festgestellt
werden muss.
Dein Aufruf
1
strcpy_P(&au8TempStr[0],AU8TXT_MAIN1);
2
lcd4x20_show(/* u8Line */0b0001,
3
/* u8Col */1,
4
/* pu8Data */&au8TempStr[0],
5
/* u8Len */strlen_P(AU8TXT_MAIN1));
verkürzt sich daher zu
1
lcd4x20_show_P(0b0001,1,AU8TXT_MAIN1);
abgesehen davon, dass dadurch der Code kürzer wird, hast du auch noch 3
zusätzliche Vorteile: Der Code ist sicherer, weil du kein Array
überlaufen kannst. Er ist schneller, weil in deiner Originalversion der
String im Flash 2-mal durchlaufen wird (1-mal für den strcpy_P und
einmal für den strlen_P) während er in der sichereren Version nur 1-mal
durchlaufen wird (bei der eigentlichen Ausgabe, die in deiner Version
aus dem Zwischenspeicher gemacht wird). Und du brauchst die Stringlänge
für den Aufruf nicht explizit bestimmen, hast also das Verwenden der
Funktion für den Aufrufer einfacher gemacht.
Hier hast du also einen Fall, in dem du durch geschicktes Einführen von
Funktionen nur Vorteile hast. Und solche Dinge solltest du mal suchen.
Mit der Zeit kriegt man nämlich auch einen Blick dafür, welche
'Transformation' in Funktionen sich lohnen und welche nicht.
Wenn du jetzt noch ein paar #define einführst um die magischen Zahlen
0b0001 und 1 wegzukriegen
dann liest sich der Funktionsaufruf schon fast wie englischer Prosatext:
Gib den Text AU8TXT_MAIN1 auf dem Lcd in der Spalte MENUTEXT_COLUMN und
in der Zeile MENUTEXT_ROW aus.
Und damit sind die Kommentare völlig überflüssig geworden! Und das ist
gut, denn diese Kommentare sind von der Sorte: sie erzählen mir nichts,
was nicht eh schon im Code auch steht.
Dadurch vereinfacht sich zb
>Was gefällt dir daran jetzt nicht?
So ist das schon ok, in der Funktion. Nur vorher, alles im case, war
Mist.
>Gewöhn dir das & ...[0]
OK.
>Die tatsächliche Länge eines Strings
Der Funktion lcd4x20_show keine Länge mitgeben? Hm.. Ja, könnte ich
ändern. Aber so kann ich festlegen, wielang die Ausgabe auf dem Display
sein soll (falls der Text länger ist..)
>Kopiere Strings niemals um..
Naja, da müsste ich aber die lcd_.._show Funktion komplett im Konzept
ändern..
>Wieviel Speicher reservierst du denn für die Kopie?
Eine Zeile, also 20Zeichen+Null.
>Wo liegt in deinem Code-Schnipsel das Problem?
Keins. Es ging mir nur um den vielen Code innerhalb eines case.
>noch ein lcd4*20_show_P
Sehr gute Idee. Wieso ist mir das nicht eingefallen? Werde ich tun.
>um die magischen Zahlen
Hm.. Stimmt. Das kann ich bei der Funktion, die den Cursor positioniert,
auch gleich nutzen ;-)
>Aus ursprünglich 17 Zeilen sind 3 geworden, ohne dass die Klarheit>darunter gelitten hätte
Vielen Dank für die Infos. Ich werde das entsprechend anpassen.
>Und solche Dinge solltest du mal suchen.>Mit der Zeit kriegt man nämlich auch einen Blick dafür, welche>'Transformation' in Funktionen sich lohnen und welche nicht.
Die fehlt mir eben noch. Deshalb frage ich ja meist in GCC-Forum. Bin
eben nur Hobby-C-Programmierer. Beruflich programmiere ich SPS, und da
läuft das etwas anders..
Matthias Lipinsky wrote:
>>Was gefällt dir daran jetzt nicht?>> So ist das schon ok, in der Funktion. Nur vorher, alles im case, war> Mist.
Man könnte sich jetzt noch Verallgemeinerungen überlgen.
Wenn ich Zeit hab, schreib ich heut am Abend noch was drüber.
>>Die tatsächliche Länge eines Strings> Der Funktion lcd4x20_show keine Länge mitgeben?
Yep.
> Hm.. Ja, könnte ich> ändern. Aber so kann ich festlegen, wielang die Ausgabe auf dem Display> sein soll (falls der Text länger ist..)
Frage: Machst du das irgendwo?
Wenn nein: Du hast hier eine mögliche Flexibilität, die im Grunde
niemand benutzt. Der Preis, den du dafür zahlst ist: Alle anderen
Ausgaben, die dieses Feature nicht benutzen, zahlen dafür mit Laufzeit.
Auch ein strlen ist nicht kostenlos!
Das errinnert mich an einen endlos komplizierten Mechanismus den mein
Kollege und ich vor Jahren in unser Programm eingebaut haben. Wir haben
in Summe 2 Mannmonate dafür verbraucht. In 15 Jahren hat niemand je die
Flexibilität benötigt, die wir ermöglicht haben :-)
In deinem Fall ist es ähnlich. Der Regelfall wird sein, dass ein String
komplett auszugeben ist. Und falls das mal nicht so sein sollte, ist es
für den Aufrufer kein Problem, sich zunächst mal einen Teilstring in der
gewünschten Länge zu bilden und den dann wieder der allgemeinen Funktion
zu übergeben.
>>>Kopiere Strings niemals um..> Naja, da müsste ich aber die lcd_.._show Funktion komplett im Konzept> ändern..
Yep. Sollte aber nicht so schwer sein. Normalerweise macht die ja nichts
anderes als den String in einzelnen Zeichen aufs Display schaufeln.
>>Wieviel Speicher reservierst du denn für die Kopie?> Eine Zeile, also 20Zeichen+Null.
Was wenn der auszugebende Text länger ist?
Der springende Punkt ist: Du hast dir hier mit diesem Zwischenarray
Inflexibilität aus keinem guten Grund eingehandelt. Eine allgemeine
String Ausgabe braucht die Längenangabe normalerweise gar nicht. Die
wenigsten stringverarbeitenden Funktionen, die einen String als Eingabe
haben, benötigen die Länge.