www.mikrocontroller.net

Forum: Projekte & Code suche Kritik zum Programmierstil ;o)


Autor: *USER* (Gast)
Datum:
Angehängte Dateien:

Bewertung
0 lesenswert
nicht lesenswert
So - habe eben mal meine erste in Anführungszeichen sinnvolle Routine
fertiggestellt und wollte jetzt einfach von den Profis ein bisschen
konstruktive Kritik erhalten ;o)
Was mir selber noch einfällt: Man müsste den String noch einmal in
ASCII decodieren, da Umlaute wie z.B. ö nicht richtig übergeben werden.
Weiß jemand welchen Befehl ich dazu nutzen kann ?

Wers gebrauchen kann: Ist für den M16C, Compiler NC30

Autor: Matthias (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Hi

ich hab mir jetzt nur mal kurz die Formatierung angesehen. Wie man bei
C einrückt ist ja Geschmacksache. Also ob

foo()
{
    bar;
}

oder

foo(){
    bar;
}

oder

foo()
    {
    bar;
    }

oder

foo()
{
bar;
}

bleibt jedem selbst überlassen (wobei ich die letzten zwei persönlich
übel finde. Aber man sollte das einheitlich machen. Also immer nur
einen Stil verwenden und nicht mischen.

Auch ein paar Leerzeilen würden die Übersicht erhöhen.

Jmy2(€/100)

Matthias

Autor: *USER* (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Au ja hast recht mit den Leerzeilen - sieht im richtigen Editor (Proton)
mit Syntax-Highlighting (oder wie das heisst) noch nicht mal mehr halb
so schlimm aus ;o)

Autor: *USER* (Gast)
Datum:
Angehängte Dateien:

Bewertung
0 lesenswert
nicht lesenswert
so - besser ? g

Nein wollte vor allem noch wissen was man am Programm selber verbessern
kann (außer dem Aussehen)

Autor: Peter Dannegger (peda)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Die main loop fehlt, Dein Programm rennt ins Nirwana.


Peter

Autor: *USER* (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
ist klar - die main ist nur zu test da - das andere soll dann
ausgelagert werden - und mit Debugger macht das ja nix ;o)

Autor: Werner Hoch (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Hi USER,

"schöne" Programme funktionieren einfach besser.

Hier sind ein paar Stile aufgeführt:

http://www.chris-lott.org/resources/cstyle/

(*(txt+i)) kann man auch als txt[i] schreiben

mfg
werner

Autor: Peter Dannegger (peda)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
"und mit Debugger macht das ja nix"


Die Sachen, die Du nicht von Anfang an beachtest, vergißt Du später
garantiert.


Peter

Autor: *USER* (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
OK du könntest recht haben - ich werds mir merken ...

Autor: Reini (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
hey user

mir persönlich sind da zu viel absolute Zeilen drin, d.h
z.B.
...
accept(100);
p9=0;
accept(100);
p9=12;
...

es wäre etwas schöner und auch für dich vielleicht einfacher, du
würdest dir eine Header-Datei schreiben und dann die eigentlich Aktion
symbolisch ausführen:
also z.B. so
lcd.h
...
/********************************************************
  declare LCD instructions
********************************************************/
#define    clr_dsp     0x01  /* clear display */
#define   disp_on     0x0C  /* Display on */
#define   disp_c_on  0x0E  /* Display on, Cursor on */
#define   disp_b_on  0x0F  /* Display on, Cursor blinkt
#define   disp_off   0x08  /* Display off */
#define    disp_mode  0x38  /* 8 bit, 2 line, 5x7 mode */
#define    inc_entry  0x06  /* increment entry mode */
#define    scr_entry  0x05  /* scroll entry mode */
#define   DD_RAM_AD10  0x80  /* Line 1 home
#define   DD_RAM_AD1E  0x93  /* End of line 1 */
und dann

lcd.c
...
Send_Command(disp_on);
Send_Command(clr_dsp);
...

Autor: Stefan May (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Du hast scheinbar ohne Compiler Optimierungen gearbeitet. Solche
Schleifen werden wegoptimiert.

for(t=0;t<1000000;t++);

Abhilfe:

1) for(t=0;t<1000000;t++) asm(" nop");
2) volatile long t; for(t=0;t<1000000;t++);

ciao, Stefan.

Autor: Hans (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
//////////snip//////////

1) for(t=0;t<1000000;t++) asm(" nop");

//////////snip//////////

sollte doch auch wegoptimiert werden oder????

sollte das nicht 'volatile asm("nop")' sein???
ich sollte wieder mal die gcc docu mir zu gemüte führen ...

73 de oe6jwf

Autor: Stefan May (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Du hast recht.

Also ich habe das jetzt mal getestet und es wird nicht wegoptimiert.
Ein volatile vor dem asm ist syntaktisch nicht korrekt, das gehört nach
dem asm hin. Also so:

for(t=0;t<1000000;t++) asm volatile (" nop");

Ich habe jetzt mal ein paar andere Testfälle durchgespielt. Wenn t als
long definiert ist, wird die Schleife nicht wegoptimiert. Scheinbar
werden nur Fälle kleiner "int" optimiert. Darauf sollte man sich aber
nicht verlassen und t entsprechent volatile definieren oder, als
schlechtere Variante, den asm volatile(" nop") einfügen.

Optimierungen sind schon ein Fall für sich. Kann einen manchmal
verzweifeln lassen. :-)

ciao, Stefan.

Autor: Rolf F. (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Mir ist dabei wichtig, dass es von einem Prettyprinter/Beautifier
gemacht wird, damit z. B. nicht

for(i=1;i<10;i++);  // durch das ; hier wird die folgende Zeile nur
einmal ausgeführt
  a+=i;             // falsch eingerückt

stehen bleibt.

Deshalb benutze ich dafür ein Skript mit dem indent mit über 10
Optionen u. anschließend einem kleinen C-Programm, das Tabs in 8
Leerzeichen wandelt u. alle Whitespaces am Zeilenende löscht.

Autor: Werner Hoch (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
@Rolf:
Dein "Verfahren" hört sich etwas aufwendig an.

hast du mal daran gedacht einfach einen anderen Editor zu nehmen,
der diese Features eingebaut hat?

Emacs z.B.
- rückt syntaxrichtig ein
- weigert sich falsch einzurücken.
- Klammerung wird geprüft

mfg
Werner

Autor: Rolf F. (Gast)
Datum:

Bewertung
0 lesenswert
nicht lesenswert
Ich benutze das zusätzlich mit dos2unix/unix2dos und für alle Dateien im
aktuellen Verzeichnis; mit emacs wäre das wohl nicht schnell machbar.
Die Klammerung wird vom Skript grob schon durch den indent geprüft.
Auch deshalb lasse ich so ein Skript über jeden Sourcecode bevor ich
mir den ansehe, denn man sieht dann schon Welche Dateien nur angefangen
und noch nie compiliert wurden.
Beim Linux-Kernel sind das nämlich einige hundert (neben den
offiziellen Kernel-Dateien, angezeigt bekommt man die errors mit "find
-name \*.[ch] -type f -print0 | xargs -0 indent | grep error") und
nach Murphy's Law sind das genau die Gerätetreiber, die man am
Dringensten für einen embedded Rechner braucht.

Antwort schreiben

Die Angabe einer E-Mail-Adresse ist freiwillig. Wenn Sie automatisch per E-Mail über Antworten auf Ihren Beitrag informiert werden möchten, melden Sie sich bitte an.

Wichtige Regeln - erst lesen, dann posten!

  • Groß- und Kleinschreibung verwenden
  • Längeren Sourcecode nicht im Text einfügen, sondern als Dateianhang

Formatierung (mehr Informationen...)

  • [c]C-Code[/c]
  • [avrasm]AVR-Assembler-Code[/avrasm]
  • [code]Code in anderen Sprachen, ASCII-Zeichnungen[/code]
  • [math]Formel in LaTeX-Syntax[/math]
  • [[Titel]] - Link zu Artikel
  • Verweis auf anderen Beitrag einfügen: Rechtsklick auf Beitragstitel,
    "Adresse kopieren", und in den Text einfügen




Bild automatisch verkleinern, falls nötig
Bitte das JPG-Format nur für Fotos und Scans verwenden!
Zeichnungen und Screenshots im PNG- oder
GIF-Format hochladen. Siehe Bildformate.
Hinweis: der ursprüngliche Beitrag ist mehr als 6 Monate alt.
Bitte hier nur auf die ursprüngliche Frage antworten,
für neue Fragen einen neuen Beitrag erstellen.

Mit dem Abschicken bestätigst du, die Nutzungsbedingungen anzuerkennen.