Bit-Einstellung und Code-Lesbarkeit

Question

Ich habe eine Arduino-Anwendung (naja, eigentlich eine Bibliothek), die eine Reihe von Status-Flags enthält - und ursprünglich habe ich sie einfach als Ints deklariert (gut uint8_t also 8 Bit unsigned Zeichen in diesem Fall). Aber ich hätte sie alle zu einer ganzen Zahl kombinieren und Bitmask-Operationen verwenden können, um den Status zu setzen und zu testen.

Ein Beispiel für ersteres:

if (_shift == HIGH) 
{ 
    _shift = LOW; 
} 
else 
{ 
    _shift = HIGH; 
} 

ein Beispiel für die letztere

#define SHIFT_BIT 0 

if (bitRead(_flags, SHIFT_BIT) == HIGH) 
{ 
    bitWrite(_flags, SHIFT_BIT, LOW); 
} 
else 
{ 
    bitWrite(_flags, SHIFT_BIT, HIGH); 
} 

Erstere liest besser, aber die letztere ist effizienter (Raum und Zeit). Sollte in dieser Situation die Raum- und Zeiteffizienz immer gewinnen oder ist dies eine Art Optimierung, die nur bei Bedarf stattfinden sollte?

(Added)

Der Vollständigkeit halber hier die Verdrahtung Definition dieser bitWrite usw. Makros:

#define bitRead(value, bit) (((value) >> (bit)) & 0x01) 
#define bitSet(value, bit) ((value) |= (1UL << (bit))) 
#define bitClear(value, bit) ((value) &= ~(1UL << (bit))) 
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit)) 

Answer 1

Check out Raymond Chen's excellent take zu diesem Thema. Zusammenfassend müssen Sie einige detaillierte Berechnungen durchführen, um herauszufinden, ob der letztere Fall tatsächlich effizienter ist, abhängig davon, wie viele Objekte vorhanden sind und wie viele Callsites diese Zustände tatsächlich setzen.

Soweit Lesbarkeit, sieht es so aus, als ob Sie dies mit Member-Variablen tun, was bedeutet, dass Sie wahrscheinlich in nette Funktionen gekapselt haben. In diesem Fall ist mir die Lesbarkeit nicht so wichtig, da zumindest der Code für die Leute, die die Klasse verwenden, gut aussieht. Sie können es jedoch immer in private Funktionen einkapseln, wenn es sich um ein Problem handelt.

Answer 2

Es kann klarer sein, wenn Sie die Notwendigkeit entfernen, die Konstanten HIGH und LOW zu verwenden, indem Sie in zwei Methoden aufteilen. Machen Sie einfach bitSet und bitClear Methoden. bitSet setzt das Bit auf HIGH, und bitClear setzt das Bit auf LOW. Dann wird es:

#define SHIFT_BIT 0 

if (bitRead(_flags, SHIFT_BIT) == HIGH) 
{ 
    bitClear(_flags, SHIFT_BIT); 
} 
else 
{ 
    bitSet(_flags, SHIFT_BIT); 
} 

Und natürlich, wenn Sie nur HIGH == 1 und LOW == 0, dann brauchen Sie nicht die == überprüfen.

Answer 3

Zu meinem Auge ist sogar Ihr letzter Code immer noch gut lesbar. Indem Sie jedem Ihrer Flaggen einen Namen geben, kann der Code ohne viel Aufwand gelesen werden.

Ein schlechter Weg, es zu tun wäre, „Magie“ Zahlen zu verwenden:

if(_flags | 0x20) { // What does this number mean? 
    do_something(); 
} 

Answer 4

Für Bitfelder, es besser ist, logische Operationen zu verwenden, so dass Sie tun können:

if (flags & FLAG_SHIFT) { 
    flags &= ~FLAG_SHIFT; 
} else { 
    flags |= FLAG_SHIFT; 
} 

Diese jetzt hat das Aussehen der ersteren mit der Effizienz der letzteren. Nun könnte man Makros anstatt Funktionen, so (wenn ich das richtig haben - wäre es so etwas wie):

#define bitIsSet(flags,bit) flags | bit 
#define bitSet(flags,bit) flags |= bit 
#define bitClear(flags,bit) flags &= ~bit 

Sie nicht den Aufwand der Aufruf von Funktionen haben, und der Code wird besser lesbar nochmal.

Ich habe nicht mit Arduino (noch) gespielt, aber es könnte schon Makros für diese Art von Sache sein, ich weiß es nicht.

Answer 5

Abhängig von der Kompatibilität des AVR-GCC-Compilers, über den ich mich nicht sicher bin, können Sie so etwas machen und die Dinge schön und sauber halten.

struct flags { 
    unsigned int flag1 : 1; //1 sets the length of the field in bits 
    unsigned int flag2 : 4; 
}; 

flags data; 

data.flag1 = 0; 
data.flag2 = 12; 

if (data.flag1 == 1) 
{ 
    data.flag1 = 0; 
} 
else 
{ 
    data.flag1 = 1; 
} 

Wenn Sie auch sofort Zugriff auf die gesamte Flagge int wollen, dann:

union 
{ 
    struct { 
     unsigned int flag1 : 1; //1 sets the length of the field in bits 
     unsigned int flag2 : 4; 
    } bits; 
    unsigned int val; 
} flags; 

Sie können dann ein wenig mit 2 Dereferenzierungsebenen Zugang: variable.bits.flag1 < --returns einzelnes Bit-Flag oder mit einer einzigen Ebene, um den gesamten int-Wert von Flags zu erhalten: variable.val < --returns int

Answer 6

Wenn Sie nicht optimieren müssen, tun Sie es nicht und verwenden Sie die einfachste Lösung.

Wenn Sie optimieren müssen, sollten Sie wissen, was:

• Die erste Version minimal schneller sein würde, wenn Sie nur das Bit setzen oder löschen, anstatt sie von Makeln, weil dann nicht Sie tun muss die Erinnerung lesen.

• Die erste Version ist besser w.r.t. Nebenläufigkeit. In der zweiten haben Sie lesen-ändern-schreiben, so müssen Sie sicherstellen, dass das Speicherbyte nicht gleichzeitig zugegriffen wird. Normalerweise würden Sie Interrupts deaktivieren, was Ihre Interrupt-Latenz etwas erhöht. Außerdem kann das Vergessen von Interrupts zu sehr unangenehmen und schwer zu findenden Fehlern führen (der bösartigste Fehler, dem ich bisher begegnet bin, war genau dieser Art).

• Die erste Version ist minimal besser w.r.t. Code-Größe (weniger Flash-Nutzung), da jeder Zugriff ein einzelner Lade- oder Speichervorgang ist. Der zweite Ansatz benötigt zusätzliche Bitoperationen.

• Die zweite Version verwendet weniger RAM, besonders wenn Sie viele dieser Bits haben.

• Die zweite Version ist auch schneller, wenn Sie mehrere Bits gleichzeitig testen möchten (z. B. eines der gesetzten Bits).

Answer 7

Wenn Sie die Lesbarkeit im Gespräch sind, Bit-Sets und C++, warum ich etwas nicht finden, auf std::bitset da drin? Ich verstehe, dass das Embedded-Programmierer-Rennen mit Bitmasken ziemlich vertraut ist und eine Blindheit für seine schiere Hässlichkeit entwickelt hat (die Masken, nicht die Rennen :), aber abgesehen von Masken und Bitfeldern hat die Standard-Bibliothek auch eine ziemlich elegante Lösung.

Ein Beispiel:

#include <bitset> 

enum tFlags { c_firstflag, c_secondflag, c_NumberOfFlags }; 

... 

std::bitset<c_NumberOfFlags> bits; 

bits.set(c_firstflag); 
if(bits.test(c_secondflag)) { 
    bits.clear(); 
} 

// even has a pretty print function! 
std::cout << bits << std::endl;// does a "100101" representation. 

Answer 8

Ich würde sagen, das erste, was ich besorgt bin mit ist: „#define SHIFT 0“ Warum nicht die Verwendung eines konstanten machen eher als ein Makro? Soweit es die Effizienz betrifft, erlaubt die Konstante, den Typ zu bestimmen, wodurch sichergestellt wird, dass danach keine Umwandlung mehr erforderlich ist.

Für die Effizienz Ihrer Technik: - zuerst, loswerden der else-Klausel (warum setzen Sie das Bit auf HIGH, wenn sein Wert bereits HIGH ist?) - Zweitens, lieber etwas lesbares zuerst haben, inline-Setter/Getter mit Bit-Maskierung intern würde die Arbeit tun, effizient und lesbar sein.

Wie für den Speicher, für C++ würde ich dazu neigen, ein Bitset (kombiniert mit einem Enum) zu verwenden.

Answer 9

Ist es zu einfach zu sagen:

flags ^= bit;