Le réglage du bit et la lisibilité du code

Question

J'ai une application Arduino (bien en fait une bibliothèque) qui a un certain nombre de drapeaux d'état dans - et à l'origine je les a simplement déclaré que ints (et donc uint8_t 8 bits non signé dans ce cas caractères). Mais je aurais pu les combinés tout en un seul entier et utilisé des opérations bitmask pour définir et tester l'état.

Un exemple de l'ancien:

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

un exemple de ce dernier

#define SHIFT_BIT 0

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

L'ancien lit mieux, mais celui-ci est plus efficace (l'espace et le temps). l'efficacité de l'espace et le temps devrait toujours gagner dans cette situation ou est-ce une sorte d'optimisation qui ne devrait se produire en cas de besoin?

(Ajouté)

Pour être complet est ici la définition de câblage de ces bitWrite etc. macros:

#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

Consultez Raymond Chen prendre un excellent sur ce problème. En résumé, vous devez faire un calcul détaillé pour savoir si ce dernier cas est en réalité plus efficace, selon le nombre d'objets par rapport à il y a combien de callsites effectivement mis ces états.

En ce qui concerne la lisibilité, il semble que vous faites cela avec des variables membres, ce qui signifie que vous avez probablement encapsulées dans de belles fonctions. Dans ce cas, je ne suis pas aussi préoccupé par la lisibilité car au moins le code pour les personnes qui utilisent la classe semble agréable. Cependant, vous pouvez toujours encapsuler dans des fonctions privées si elle est une préoccupation.

Answer 2

En fonction de la conformité du compilateur AVR-GCC, que je ne suis pas sûr, vous pouvez faire quelque chose comme ça et garder les choses agréable et propre.

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

Si vous souhaitez également l'accès à l'ensemble de drapeau int à la fois, puis:

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

Vous pouvez ensuite accéder à un peu avec 2 niveaux de indirection: variable.bits.flag1 <- retourne drapeau un seul bit ou avec un seul niveau pour obtenir toute la valeur int de drapeaux: variable.val <- renvoie un int

Answer 3

Il peut être plus clair si vous supprimez la nécessité d'utiliser des constantes HIGH et LOW, en se divisant en deux méthodes. Il suffit de rendre les méthodes de bitSet et bitClear. bitSet définit le bit à HIGH et bitClear définit le bit à LOW. Il devient alors:

#define SHIFT_BIT 0

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

Et bien sûr, si vous avez juste HIGH == 1 et LOW == 0, vous n'avez pas besoin du contrôle de ==.

Answer 4

A mes yeux, même votre dernier code est encore tout à fait lisible. En donnant un nom à chacun de vos drapeaux, le code peut être lu sans trop d'effort.

Une mauvaise façon de le faire serait d'utiliser des « nombres magiques »:

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

Answer 5

Si vous n'avez pas besoin d'optimiser, ne pas le faire et utiliser la solution la plus simple.

Si vous avez besoin d'optimiser, vous devez savoir pour quoi:

• La première version serait peu plus rapide si vous ne définissez ou effacer le bit au lieu de basculer, parce que vous n'avez pas besoin de lire la mémoire.

• La première version est meilleure w.r.t. concurrency. Dans le second, vous avez lecture-modification-écriture, de sorte que vous devez vous assurer que l'octet de mémoire n'est pas accessible en même temps. En général, vous pouvez désactiver les interruptions, ce qui augmente votre temps d'attente d'interruption un peu. De plus, en oubliant de désactiver les interruptions peut conduire à de très méchant et difficile de trouver des bugs (bug le plus méchant que j'ai rencontré à ce jour était exactement ce genre).

• La première version est peu mieux w.r.t. la taille du code (utilisation moins flash), parce que chaque accès est une seule opération de chargement ou un magasin. La seconde approche a besoin d'opérations de bits supplémentaires.

• La deuxième version est utilise moins de RAM, surtout si vous avez beaucoup de ces bits.

• La deuxième version est également plus rapide si vous voulez tester plusieurs bits à la fois (par exemple est l'un des bits set).

Answer 6

Si vous parlez de lisibilité, bit-ensembles et C ++, pourquoi ne pas trouver quoi que ce soit sur std::bitset là-dedans? Je comprends que la course de programmeur intégré est tout à fait à l'aise avec bitmasks, et a évolué un aveuglement pour sa laideur pure (les masques, pas les courses :), mais en dehors des masques et bitfields, la bibliothèque standard a une solution assez élégante, aussi.

Un exemple:

#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

Pour les champs de bits, il est préférable d'utiliser des opérations logiques, de sorte que vous pourriez faire:

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

Cela a maintenant le regard de l'ancien avec l'efficacité de ce dernier. Maintenant, vous pourriez avoir des macros plutôt que des fonctions, donc (si j'ai ce droit - ce serait quelque chose comme):

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

Vous n'avez pas les frais généraux de l'appel de fonctions, et le code devient plus lisible à nouveau.

Je n'ai pas joué avec Arduino (encore) mais peut-être déjà des macros pour ce genre de chose, je ne sais pas.

Answer 8

Je dirais que la première chose que je suis préoccupé par est: "#Define SHIFT 0" Pourquoi ne pas faire usage d'une constante plutôt qu'une macro? En ce qui concerne l'efficacité obtenir, la constante permet de déterminer le type, assurant ainsi que la conversion ne sera nécessaire par la suite.

En ce qui concerne l'efficacité de votre Technic: - Tout d'abord, se débarrasser de la clause else (pourquoi mettre le bit à HIGH si sa valeur est déjà élevée?) - deuxièmement, préfèrent avoir quelque chose facile à lire d'abord, setters inline / getters en utilisant peu de masquage interne serait faire le travail, être efficace et être lisible

.

En ce qui concerne le stockage, le C ++ je tendance à utiliser un bitset (en combinaison avec un enum).

Answer 9

Est-il trop simple il suffit de dire:

flags ^= bit;