Guten Morgen!!

Ich hab da mal eine Frage zum Programmierstil. Ich programmiere jetzt
schon eine ganze Weile in C für AVR und bin schon fast ein Experte
darin. Es funktioniert alles, nur bin ich nicht sicher, ob mein Stil
effizient oder übersichtlich ist.

Meine Frage bezieht sich auf folgenden Sachverhalt:

Ich habe ein Register (z.B. das OCR0A), dessen Wert ich an
unterschiedlichen Stellen im Programm und aus unterschiedlichen C-Files
aufrufe.

Welche Variante ist besser?

Variante 1:

Das Register jedesmal direkt ändern:

...
OCR0A = value1;
...
OCR0A = value2;
...

Variante 2:

Funktion, um Register zu ändern:

void Set_Reg(uint8_t val)
{
 OCR0A = val;
}
...
Set_Reg(value1);
...
Set_Reg(value2);
...

Wenn ich eine Variable in einem C-File habe, die aber aus einem andern
C-File ändern will, bleibt mir ja nichts anderes übrig, als eine
Funktion zu machen und die dann über ein h-File zu includieren, oder?

Wie macht ihr das?

Danke für Eure Antworten
Gruss
Patrick

Variante 2 ist auf jeden Fall die schönere,
wenn du z.B. einen anderen Prozessor verwendest ist eine Änderung viel
leichter
(auch wenn ich aus Bequemlichkeit oft Vraiante 1 verwende)

>Wenn ich eine Variable in einem C-File habe, die aber aus einem andern
>C-File ändern will, bleibt mir ja nichts anderes übrig, als eine
>Funktion zu machen und die dann über ein h-File zu includieren, oder?
du kannst doch im zweiten File die Variable als extern deklarieren

Mit Variante 2 gewinnst du rein gar nichts. Etwas anderes ist es, wenn
der Funktionsname die eigentliche Aufgabe beschreibt. So wie z.B.

void led_on()
{
    PORTB5 &= ~(1 << PB5);
}

Am wichtigsten erachte ich an einem guten Programmierstil das der
Programmmierer sich über die Konsequenzen des jeweilig angewendeten
Stils im klaren ist und den optimalsten auch verwendet.

Variante 2. mag zwar am schönsten sein ist aber auf AVRs zb. meist
untauglich. Würde man zb. diese Variante benutzen und in einer ISR
verwenden (was der wahrscheinlichste Fall ist) und benutzt zb. WinAVR
GCC so produziert der Compiler eine so enormen Codeoverhead das das
Laufzeitverhalten der ISR gefährdet ist. Denn bei jedem Aufruf eine
Unterfunktion innerhalb einer ISR muß der Compiler alle Register auf dem
Stack retten. Er weis ja nicht welche Register in den Unterfunktionen
benutzt werden.

Also wäre Variante 3. noch am besten

#define Set_Reg(Value)   OCR0A = Value;

Stur, nur weil es Usus ist, ein Programmierstil quasi wie ein Program
abzufahren ist der schlechteste Stil. Ein guter Stil zeichnet sich durch
Explizität und dem Wissen darum was man tut aus ohne unübersichtlich zu
wirken.

Variante 2. produziert massenhaft ineffizienten Code und das nur aus dem
Grunde weil der programmierer meint es wäre guter Programmierstil es so
zu tuen. Das ist kontraproduktiv und das ist kein guter Stil.

Gruß Hagen

Variante 2, aber das ganze als Macro oder inline-function.

Sonst hast du jedesmal den overhead für den Funktionsaufruf und
Rücksprung.

Stimme Hagen Re voll zu. Am wichtigsten ist, dass man sich überlegt,
welches Ziel man erreichen möchte.

Habt ihr vielleicht einen kleinen Styleguide mit Programmierrichtlinien,
die ihr hier veröffentlichen könnt?

Inline Funktion wäre die Lösung wenn auch alle Compiler da gleich
reagieren würden. Tun sie aber nicht.

Ergo würde ich den Precompiler benutzen und somit Makros.

Gruß Hagen

Trotzdem, was gewinnst man mit Variante 2? Meiner Meinung nach verliert
man eher was. Denn wenn ich schreibe

Set_Reg=0x12

habe ich nach zwei Monaten keine Ahnung mehr, welches Register ich jetzt
genau setze. Wenn ich die Routine dagegen Set_OCR0A() nenne, kann ich
auch gleich

OCR0A=0x12

schreiben.

Sinnvoll ist meiner Meinung nach nur die Variante, wie Rolf Magnus sie
vorschlägt. So mache ich es selbst auch, natürlich als Macro. Man kann
so auch wunderbar Ports abfragen. Beispiel:

#define State_Sw_1 (PORTA&(1<<PA2))

if(State_Sw_1)
...

Oh man, zwei Zeilen C-Code und gleich ein Fehler drin ;-)

Naja das ist ja die Variante 3. die ich mal eben so noch hinzugefügt
habe ;)
Über den Precompiler per Makros zu gehen halte ich für am
geschicktesten, auf einem AVR und den zur Verfügung stehenden Compilern.
Natürlich mit sprechenden Funktionsnamen, das ist klar. Set_Reg() ist
eine Verklausulierung die nicht explizit ist, also exakt das aussagt was
sie macht. Sowas ist generell ein schlechter Stil egal welche Variante
man nun benutzt.

Das explizite Auflösen nach OCR0A = 0x12; kann uU. eben deshalb schlecht
sein weil es an mehreren Stellen im Source erfolgt. Bei einer
Portierung, zb. auf einen anderen AVR bei dem wir nun Timer1 nutzen
müssen, muß der gesammte Source überarbeitet werden.

Würde man stattdessen

#define Set_OCR(Value)  OCR0A = Value;

benutzen, so braucht man nur noch dieses #Define zu ändern. Gerade bei
Kommunikations-/GLCD Blibliotheken vereinfacht das die Portierung enorm.
Davon abgesehen kann man nun diesen Makros Namen geben die mehr die
inhaltliche Funktion umschreiben als die Resource die man benutzt.

Also dann zb.

#define Set_Duty_Cycle(Value)  OCR0A = Value;

Somit hat der Programmierer nicht nur eine Resource (Timer0 und OCR0A)
effizient genutzt sondern auch das inhaltliche Ziel der Funktion auch
explizit beschrieben.

Gruß Hagen

Set_Reg sollte auch nur ein Beispiel sein. Der Name ist mir so spontan
eingefallen. Normalerweise würde ich z.B. schreiben:

Set_Speed
oder
SetMotorState
oder so.

Wenn ich ein Macro in ein header-file packe, kann ich ja von anderen
C-Files darauf zugreifen. Wird dann aber immer die gleiche Variable
verändert, oder wie muss ich die dann deklarieren?

Also z.B.:

#define SetMotorState(state) MotorState = state
static volatile uint8_t MotorState;

im header-file.

Jain ;) Das sind die Fallstricke des C. Es hängt davon ab wie du die
Header Files einbindest und wo du die globalen Variablen deklarierst.
Normalerweise sollten solche Variablen, auf die die Makros zugreifen ja
separat im C File drinnen stehen. Ich persönlich bevorzuge den ansich
unsauberen Weg sie im Header File zu deklarieren. Grundsätzlich gehören
sie ja auch zur jeweiligen Bibliothek die aus einem Header und C File
bestehen.

Makros sind Precompiler Directives, also nichts anderes als qausi
"Textformatierungen" die vor dem eigentlichen Übersetzen des C Source
diesen C Souce expandieren/erweitern. Je nachdem welchen C Source man
nun compiliert  werden die aktuell gültigen Makros darauf angewendet.
Wenn also in 2 verschiedenen C Souces jeweil die Variable MachineState
deklariert wurde und auch das Makro SetMachineState() benutzt wird so
versucht der Precompiler auch dieses Makro darauf anzuwenden. Auch wenn
im Grunde der Programmierer nur für das 1. C File dieses Verhalten
vorgesehen hat und nicht für das 2. C File.

Gruß Hagen

Würde das so funktionieren:
Ich habe zwei files. Timer.c und motor.c. Und das jeweilge H-File.

Nun steht in Timer.h folgendes:

#define Reset_Cnt cnt=0

volatile unsigned char cnt;

In Timer.c:

ISR (TIMER1_OVF_vect)
{
 cnt++;
 if (cnt == 5)
 {
  ...
  Reset_Cnt;          //reset counter
 }
}

Jetzt steht in motor.c:

#include Timer.h

void Irgendwas (...)
{
 ...
 Reset_Cnt;
}

Ja, da die globale Variable in Timer.h deklariert wurde. Sie darf aber
nicht in Motor.c nochmals dekaliert sein, dann würde nämlich Reset_Cnt;
diese Variable statt die in Timer.h zurücksetzen. Das sind ja die
Fallstricke von denen ich sprach. Normalerweise würde der Compiler aber
eine Warnung ausspucken um genau auf diesen Sachverhalt hinzuweisen. Es
liegt also auch in der Verantwortung des Programmierers. Sauber wäre es
wenn diese globale Variable in Timer.c als lokale Variable deklariert
wäre. Dann kann ein Makro diese nicht mehr von extern verändern.
Allerdings benötigst du dann Zurgiffsfunktionen -> Setterfunction, so
wie deine Variante 2 und die hätte auf einen AVR eben auch gravierende
Nachteile, zb. wenn in ISR verwendet.

Allerdings ist dein Beispiel auch schlecht gewählt. Deine Timer.c greift
ja direkt auf cnt zu und hat damit nicht die Konsequenzen zu fürchten
die bei nicht inlined Functions drohen. Du kannst dann so vorgehen

volatile unsigned char cnt = 5

void reset_counter(void) {

  cnt = 5;
}

ISR (TIMER1_OVF_vect)
{
 if (!(--cnt))
 {
  ...
  cnt = 5;
 }
}

cnt ist dann lokal sichtbar in Timer.c deklariert und muß aus anderen C
Sourcen über reset_counter() zurückgesetzt werden. Ich habe mal die
Zählweise umgeschrieben da der WinAVR GCC damit besseren Code erzeugt.

Gruß Hagen

Der wichtigste Part bei Programmieren ist wohl eher, dass man in der
Lage ist, seinen Code sauber zu dokumentieren. Bei winzigen bis kleinen
Projekten
(nein ich schreib jetzt nicht wiviele Zeilen Code oder wieviele Dateien
;) )

kann man relativ problemlos nach dem Motto "Quick an dirty"
programmieren.
Also die Variante:

OC1A = <Wert>;

Bei großen, Modularen Projekten ( AVR mit Webserver, DCF77, etc. )
schreibt man sich für die Hardwarenahen Funktionen vorzugsweise ein HAL
(Hardware Abstraction Layer). Ausserdem würde ich in so einem Fall NIE
eine Variable extern deklarieren, wenn es nicht tausend Verenkungen nach
sich zieht, das nicht zu machen! Immer alle kapseln und Wertänderungen /
Abfragen per Zugriffsfunktion. Dann gibt es keinen Salat.

Es wäre allerdings ziemlich kurzsichtig, wenn ich nicht erwähnen würde,
dass
es auch Probleme in der Realität gibt, die sich aus Performancegründen
nicht anders realisieren lassen (weiter oben wurde das schonmal
erwähnt).

Beispiel für HAL:

io.h
#define ON   1 // oder auch true
#define OFF  0 // oder auch false

#define RELAIS_2 5
io.c

setOutput( byte select, bool state )
{
 switch( select )
 ...
 case RELAIS_2:
   if( state ) PORTX |= 0x02;
   else        PORTX &= ~(0x02);
   break;
 ...
}


andere.c

setOutput( RELAIS_2, ON );


Das lässt sich auch auf andere Bereiche in ähnlicher Form anwenden.
Zu empfehlen wäre auch ein Source Doku System z.B. Doxygen, damit man
die Übersicht behält. Andernfalls weiss man ein paar Wochen später
nimmer was man programmiert hat ;)

Stimme da Matthias zu. Das Wichtigste an einem guten Stil ist das man
durch Abwägen einen Komprmiss schafft zwischen Zielsetzung, Aufwand,
Möglichkeiten der Hardware, Werkzeuge wie Compiler/Dokusystem usw. Einen
starren Programmierstil dafür zu benutzen ist also kontraproduktiv,
defakto gibt es aus dieser Sichtweise heraus keinen guten oder
schlechten Programmierstil sondern einen weniger oder mehr effizienten
und effektiven Programmierstil.

Gruß Hagen

Hallo,

habe den Thread gerade erst gelesen.

Ich verwende globale Variablen, die in mehreren Dateien verwendet werden
sollen so, dass ich in der h-Datei Deklaration und Definition habe.
Beide werden mit #ifdef getrennt.

Das ganze sieht dann so aus.

h-Datei:
#ifdef PLACE_GLOBALS_HERE
   struct Glob_Beispiel status;
#else
   extern struct Glob_Beispiel status;
#endif

Die h-Datei kann jetzt in allen C-Dateien eingebunden werden, wo sie
benötigt wird.
In main.c füge ich vor dem #include noch ein #define PLACE_GLOBALS_HERE
ein.

Somit habe ich im Anpassungsfall nur eine Datei zu ändern.

Santiago wrote:

> Somit habe ich im Anpassungsfall nur eine Datei zu ändern.

Solange bei den Variablen keine Initialisierung gemacht wird,
kann man das Ganze auch noch so vereinfachen:

h-Datei:
********

#ifndef EXTERN
#define EXTERN extern
#endif

EXTERN struct Glob_Beispiel status;

In allen *.c Files (ausser main) wird das Include File einfach
wie gehabt eingebunden.
In der main.c sieht das dann so aus:

#define EXTERN
#include "h-Datei"

....

Das #define EXTERN sorgt dann dafür, dass sich das Header File
nicht EXTERN auf extern umdefiniert und aus den Deklarationen
werden Definitionen.
Vorteil: Noch weniger Tipparbeit und da Variablennamen nur
einmal vorkommen, können hier auch keine Tippfehler entstehen
sodass Definition und Deklaration nicht mehr zusammenpassen.

Geht aber wie gesagt nur dann, wenn keine Initialisierung im
Spiel ist.

Die Frage ist allerdings immer, ob man die Variable wirklich quer durch
das ganze Programm schleifen will. Bei einigen Controllern bleibt einem
nichts anderes übrig, da man mit Speicher knausern muss.

Eine Kapselung, ähnlich einer c++ klasse, nur ohne "struct" und "class"
dürfte bei großen, modularen Projekten erheblich zur Übersichtlichkeit
beitragen.

Dieser Thread ist mal wieder das beste Beispiel dafür, dass
Programmierstil absolute Geschmackssache ist (Okay, neben ein paar
Wirklich einleuchtenden Punkten, was man wirklich nicht machen sollte).

Mein Tipp: Schau dir den Programmierstil anderer Leute an. Mglw. Leute,
die schon länger programmieren und das evtl. auch beruflich machen.
Schau dir Sachen ab, die dir gefallen oder die du sogar praktisch
findest. Sachen, wo du aber keinen Sinn drin sehen kannst oder die keine
großartige Bedeutung haben (zum Beispiel: Schreibe ich Variablennamen
nun groß, klein mit "_"? Gewöhne ich mir an PräproMakros immer komplett
groß zu schreiben), solltest du dir so legen, wie du am besten damit
klar kommst.

Wenn du den Code nachher noch mit anderen Leuten austauschst oder
veröffentlichst, würde ich darauf achten, den eigenen Programmierstil
nicht so besonders hervorheben zu wollen. Das kann problematisch werden,
denn was dem einen gefällt ist für den anderen reines Programmierchaos.

Achtung:
---------
Bitte keinen "Lonely Hacker Stil" abkupfern. Der kommt meistens bei
Leuten vor, die noch nie im Team an einer Software geschrieben haben!

So einen Code zu lesen ist echt grausam ;-)

Ja, den Hinweis habe ich natürlich auch gerade schon gegeben. Aber die
Hauptsache an einem Programmierstil ist (sofern nicht einer vorgegeben
ist), dass man selber gut und schnell und fehlerarm programmiert.

Wenn es ans Teamwork geht sind hingegen idR schon Codestile vorgegeben,
die man einhalten muss. Und, wie bereits gesagt, wenn du das nachher
veröffentlichen willst, würde ich auch nicht die stark spezialisierte
Codestil-Seite 'raushängen lassen ;)

Achja: Ich mag's nicht code zu lesen, wo Namen (von Funktionen,
Variablen, whatever) komplett kleingeschrieben sind. Ich weiß aber, dass
es sehr weit verbreitet ist (und früher durchaus üblicher war als heute)
und gebe deswegen nicht viel drum :-)

Matthias wrote:
> Die Frage ist allerdings immer, ob man die Variable wirklich quer durch
> das ganze Programm schleifen will. Bei einigen Controllern bleibt einem
> nichts anderes übrig, da man mit Speicher knausern muss.

Nö.

Die Sichtbarkeit einer Variablen hat keinerlei Einfluß auf den
Speicherbedarf.

Ich halte nichts davon, alle globalen Variablen in ein File zusammen zu
pappen.
Ich mache immer zu jedem Modul eigenen Headerfiles. z.B. Datum und
Uhrzeit werden in timer.h bekannt gemacht und in timer.c angelegt usw.


Peter