Biteinstellung und Code -Lesbarkeit

StackOverflow https://stackoverflow.com/questions/1197111

Frage

Ich habe eine Arduino -Anwendung (tatsächlich eine Bibliothek), in der eine Reihe von Statusflags enthalten sind - und ursprünglich habe ich sie einfach als INT deklariert (in diesem Fall 8 -Bit -Signalkosten). Aber ich hätte sie alle zu einer Ganzzahl kombinieren und Bitmask -Operationen verwenden können, um den Status zu setzen und zu testen.

Ein Beispiel für das erstere:

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

Ein Beispiel für letztere

#define SHIFT_BIT 0

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

Ersteres liest besser, aber letzteres ist effizienter (Raum und Zeit). Sollte der Raum und die Zeiteffizienz in dieser Situation immer gewinnen, oder ist dies eine Art Optimierung, die nur bei Bedarf passieren sollte?

(Hinzugefügt)

Für die Vollständigkeit ist hier die Verkabelungsdefinition 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))
War es hilfreich?

Lösung

Kasse Raymond Chens ausgezeichnete Einstellung zu diesem Thema. Zusammenfassend müssen Sie eine detaillierte Berechnung durchführen, um herauszufinden, ob der letztere Fall tatsächlich effizienter ist, je nachdem, wie viele Objekte es gibt, als wie viele Callsites diese Zustände tatsächlich festlegen.

Was die Lesbarkeit angeht, so sieht es so aus, als hätten Sie dies mit Mitgliedsvariablen, was bedeutet, dass Sie sie wahrscheinlich in netten Funktionen verkörpert haben. In diesem Fall bin ich nicht so besorgt über die Lesbarkeit, da zumindest der Code für die Personen, die die Klasse verwenden, gut aussieht. Sie könnten es jedoch immer in privaten Funktionen zusammenfassen, wenn es sich um ein Problem handelt.

Andere Tipps

Abhängig von der Einhaltung des AVR-GCC-Compilers, über den ich mir nicht sicher bin, können Sie so etwas tun 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 Zugriff auf die gesamte Flagge gleichzeitig int wünschen, 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 mit 2 Indirektionsstufen ein wenig zugreifen: variable.bits.flag1<-Gibt ein Bitflag oder mit einem einzigen Level zurück, um den gesamten Int-Wert von Flags zu erhalten: variable.val <-Gibt int int zurück

Es kann klarer sein, wenn Sie die Notwendigkeit entfernen, Konstanten zu verwenden HIGH und LOW, durch Aufteilung in zwei Methoden. Mach 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 haben HIGH == 1 und LOW == 0, Dann brauchst du nicht die == Überprüfung.

Für mein Auge ist sogar Ihr letzterer Code immer noch recht lesbar. Indem der Code jedem Ihrer Flags einen Namen gibt, kann er ohne viel Aufwand gelesen werden.

Ein schlechter Weg, es zu tun, wäre, "magische" Zahlen zu verwenden:

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

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 wäre minimal schneller, wenn Sie das Bit nur einstellen oder löschen, anstatt es umzuschalten, da Sie den Speicher nicht lesen müssen.

  • Die erste Version ist eine bessere Parallelität. In der Sekunde haben Sie das Lesen-Modify-Write und müssen also sicherstellen, dass das Speicherbyte nicht gleichzeitig zugegriffen wird. Normalerweise deaktivieren Sie Interrupts, was Ihre Interrupt -Latenz etwas erhöht. Das Vergessen, Interrupts zu deaktivieren, kann zu sehr bösen und schwer zu findenden Fehler führen (der schlimmste Fehler, den ich bisher begegnet habe, war genau diese Art).

  • Die erste Version ist minimal bessere WRT -Codegröße (weniger Flash -Verwendung), da jeder Zugriff ein einzelner Last- oder Speichervorgang ist. Der zweite Ansatz erfordert zusätzliche Bitoperationen.

  • Die zweite Version wird weniger RAM verwendet, insbesondere wenn Sie viele dieser Teile haben.

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

Wenn Sie über Lesbarkeit, Bit-Sets und C ++ sprechen, warum finde ich dann nichts? std::bitset da drin? Ich verstehe, dass das eingebettete Programmierrennen mit Bitmasks ziemlich wohl ist und eine Blindheit für seine hässliche Hässlichkeit entwickelt hat (die Masken, nicht die Rennen :), aber abgesehen von Masken und Bitfields hat die Standardbibliothek 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.

Für Bit Fields ist es besser, logische Operationen zu verwenden, sodass Sie dies tun können:

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

Dies hat jetzt das Aussehen des ersteren mit der Effizienz des letzteren. Jetzt könnten Sie eher Makros als Funktionen haben, also (wenn ich das richtig habe - es wäre so etwas wie):

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

Sie haben nicht den Aufwand, Funktionen aufzurufen, und der Code wird wieder lesbarer.

Ich habe (noch) nicht mit Arduino gespielt, aber es könnte bereits Makros für so etwas geben, ich weiß nicht.

Ich würde sagen, dass das erste, was ich besorgt bin,: "#define Verschiebung 0" Warum nutzen Sie nicht eine Konstante als ein Makro? Was die Effizienz angeht, erlaubt die Konstante den Typ, den Typ zu bestimmen, wodurch danach keine Umwandlung erforderlich ist.

Was die Effizienz Ihrer Technik betrifft: - Erstens die else -Klausel (warum das Bit hoch einstellen, wenn sein Wert bereits hoch ist?) - Zweitens, lieber etwas lesbares zuerst, instrahlende Setter / Getter mit Bit -Maskierung intern zu haben würde den Job machen, effizient sein und lesbar sein.

Für den Speicher würde ich für C ++ eine Bitset (kombiniert mit einer Aufzählung) verwenden.

Ist es zu einfach, nur zu sagen:

flags ^= bit;
Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top