impostazione bit e la leggibilità del codice

Question

Ho un'applicazione Arduino (beh in realtà una biblioteca), che ha una serie di flag di stato in esso - e in origine ho semplicemente dichiarato come int (e uint8_t caratteri in modo da 8 bit senza segno in questo caso). Ma ho potuto li ho combinati in un intero e utilizzato maschera di bit operazioni per impostare e verificare lo stato.

Un esempio del primo caso:

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

un esempio di quest'ultimo

#define SHIFT_BIT 0

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

L'ex legge meglio, ma il secondo è più efficiente (spazio e tempo). Se l'efficienza dello spazio e del tempo vince sempre in questa situazione o si tratta di una sorta di ottimizzazione che dovrebbe accadere solo quando serve?

(Aggiunto)

Per completezza ecco la definizione di cablaggio di quei bitWrite ecc macro:

#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

Raymond Chen eccellente prendere su questa edizione. In sintesi, è necessario fare un po 'di calcolo dettagliato per scoprire se quest'ultimo caso è in realtà più efficiente, a seconda di quanti oggetti ci sono rispetto a quanti effettivamente callsites impostare questi stati.

Per quanto riguarda la leggibilità, sembra che si sta facendo questo con variabili membro, il che significa che probabilmente avete li incapsulati in funzioni piacevoli. In quel caso, io non sono così preoccupato con leggibilità perché almeno il codice per le persone che utilizzano la classe sembra piacevole. Tuttavia, si può sempre incapsulare in funzioni private se si tratta di una preoccupazione.

Answer 2

A seconda della conformità del compilatore AVR-GCC, che sono incerti circa, si può fare qualcosa di simile e mantenere le cose belle e pulite.

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;
}

Se si desidera anche l'accesso a tutta l'int bandiera in una sola volta, poi:

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

È quindi possibile accedere a un po 'con 2 livelli di riferimento indiretto: variable.bits.flag1 <- restituisce sola bandiera bit o con un unico livello per ottenere l'intero int vale la pena di bandiere: variable.val <- Restituisce int

Answer 3

Può essere più chiaro se si rimuove la necessità di utilizzare le costanti HIGH e LOW, dividendosi in due metodi. Basta fare bitSet e bitClear metodi. bitSet imposta il bit a HIGH, e bitClear imposta il bit a LOW. Allora diventa:

#define SHIFT_BIT 0

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

E, naturalmente, se avete appena HIGH == 1 e LOW == 0, quindi non è necessario il controllo ==.

Answer 4

Per il mio occhio, anche il vostro quest'ultimo codice è ancora abbastanza leggibile. Dando un nome a ciascuna delle vostre bandiere, il codice può essere letto senza molto sforzo.

Un buon sistema per farlo sarebbe quello di usare i numeri "magici":

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

Answer 5

Se non avete bisogno di ottimizzare, non farlo e utilizzare la soluzione più semplice.

Se si ha bisogno di ottimizzare, si dovrebbe sapere per quale:

• La prima versione sarebbe minimamente più veloce se si imposta solo o deselezionare il bit invece di commutazione, perché allora non c'è bisogno di leggere la memoria.

• La prima versione è migliore w.r.t. concorrenza. Nel secondo di aver letto con riscrittura, quindi è necessario fare in modo che il byte di memoria non si accede contemporaneamente. Di solito si dovrebbe disabilitare gli interrupt, che aumenta la latenza di interrupt un po '. Inoltre, dimenticando di disabilitare gli interrupt può portare a molto brutto e difficile da trovare bug (il bug più brutta che ho incontrato finora è stato esattamente questo tipo).

• La prima versione è minimamente meglio w.r.t. dimensione del codice (minor utilizzo flash), perché ogni accesso è una singola operazione di caricamento o negozio. Il secondo approccio ha bisogno di operazioni di bit aggiuntivi.

• La seconda versione è usa meno RAM, soprattutto se si hanno un sacco di questi bit.

• La seconda versione è anche più veloce se si vuole testare più bit in una sola volta (per esempio è uno dei bit impostati).

Answer 6

Se stai parlando leggibilità, bit-set e C ++, perché non trovo nulla sul std::bitset lì dentro? Capisco che la gara programmatore incorporato è abbastanza confortevole con maschere di bit, e si è evoluto una cecità per la sua eterea uglyness (le maschere, non le gare :), ma a parte le maschere e campi di bit, la libreria standard ha una soluzione molto elegante, anche.

Un esempio:

#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 7

Per i campi di bit, è meglio utilizzare le operazioni logiche, così si potrebbe fare:

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

Questo ha ora l'aspetto del primo con l'efficienza di questi ultimi. Ora si potrebbe avere le macro piuttosto che funzioni, in modo (se ho questo diritto - sarebbe qualcosa di simile):

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

Non è il sovraccarico di funzioni di chiamata, e il codice diventa ancora più leggibile.

Non ho giocato con Arduino (ancora), ma ci potrebbe essere già macro per questo genere di cose, non lo so.

Answer 8

Direi che la prima cosa che mi riguarda con è: "#Define SHIFT 0" Perché non fare uso di una costante, piuttosto che una macro? Per quanto riguarda l'efficienza ottenere, la costante permettono di determinare il tipo, in tal modo facendo attenzione che la conversione non sarà necessaria in seguito.

Per quanto riguarda l'efficienza della vostra tecnica: - in primo luogo, sbarazzarsi della clausola else (il motivo per cui impostare il bit su HIGH se il suo valore è già alto?) - in secondo luogo, preferiscono avere qualcosa di leggibile primi, setter inline / getter utilizzando po 'di mascheramento internamente avrebbe fatto il lavoro, essere efficienti ed essere leggibile

.

Per quanto riguarda l'archiviazione, per C ++ avrei tendono ad usare un bitset (in combinazione con un enum).

Answer 9

E 'troppo semplice solo per dire:

flags ^= bit;