Jai récemment commencé à nettoyer mes projets et jai décidé que je voulais créer un référentiel personnel de morceaux de code utiles. Jai trouvé ce code bitmap entre eux et je lai fortement édité et japprécierais que quelquun puisse lexaminer, car je ne suis pas habitué aux opérations bit à bit et autres. Le code est-il suffisamment portable ou il devrait y avoir des changements?
bitmap.h
#ifndef BITMAP__ #define BITMAP__ #define BIT (8*sizeof(byte)) #define BITMAP_NOTFOUND -1 typedef enum{false=0, true} bool; typedef unsigned char byte; bool bitmapGet (byte *, int); void bitmapSet (byte *, int); void bitmapReset (byte *, int); int bitmapSearch(byte *, bool, int, int); #endif bitmap.c
#include "bitmap.h" static bool get (byte, byte); static void set (byte *, byte); static void reset(byte *, byte); /* CAREFUL WITH pos AND BITMAP SIZE! */ bool bitmapGet(byte *bitmap, int pos) { /* gets the value of the bit at pos */ return get(bitmap[pos/BIT], pos%BIT); } void bitmapSet(byte *bitmap, int pos) { /* sets bit at pos to 1 */ set(&bitmap[pos/BIT], pos%BIT); } void bitmapReset(byte *bitmap, int pos) { /* sets bit at pos to 0 */ reset(&bitmap[pos/BIT], pos%BIT); } int bitmapSearch(byte *bitmap, bool n, int size, int start) { /* Finds the first n value in bitmap after start */ /* size is the Bitmap size in bytes */ int i; /* size is now the Bitmap size in bits */ for(i = start+1, size *= BIT; i < size; i++) if(bitmapGet(bitmap,i) == n) return i; return BITMAP_NOTFOUND; } static bool get(byte a, byte pos) { /* pos is something from 0 to 7*/ return (a >> pos) & 1; } static void set(byte *a, byte pos) { /* pos is something from 0 to 7*/ /* sets bit to 1 */ *a |= 1 << pos; } static void reset(byte *a, byte pos) { /* pos is something from 0 to 7*/ /* sets bit to 0 */ *a &= ~(1 << pos); } Commentaires
- À tous ceux qui suivent encore ce message, jai envoyé le code à github ici . Jai également ajouté tous vos commentaires en tant que problèmes. Merci pour vos commentaires et je vais commencer à les implémenter.
Réponse
Si vous pensez que votre code sera un jour utilisé à partir de C ++ (ce qui fait beaucoup de code C), vous ne devez utiliser aucun identifiant avec un double trait de soulignement.
#ifndef BITMAP__ // ^^ May cause problems in C++ Ceci est réservé à limplémentation en C ++. De plus, le terme BITMAP est très générique et vous êtes très susceptible dentrer en conflit avec dautres bibliothèques essayez de le rendre plus unique à votre projet.
Vous supposez quun caractère est de 8 bits.
#define BIT (8*sizeof(byte)) Le standard ne le dit pas (cest au moins 8 bits) mais il est en fait défini via la macro CHAR_BITS donc vous devriez utiliser:
#define BIT (CHAR_BITS*sizeof(byte)) Votre fichier den-tête est la façon dont la plupart des gens verront dabord votre code et essaieront de comprendre comment lutiliser. Ainsi, mettre les noms des paramètres dans le fichier den-tête est probablement une bonne idée car cela aide à documenter leur utilisation.
bool bitmapGet (byte *, int); void bitmapSet (byte *, int); void bitmapReset (byte *, int); int bitmapSearch(byte *, bool, int, int); En plus de laction de bitmapSearch nest pas évident donc un commentaire sur son utilisation dans len-tête serait probablement une bonne idée. En fait, une petite présentation sur la façon dont le premier paramètre peut être un tableau serait probablement une bonne idée (car ce nest pas évident sans lire le code).
Soyez prudent avec les commentaires:
/* pos is something from 0 to 7*/ Ceci nest vrai que si vous faites des hypothèses (A: that sizeof (byte) == 1 B: vous multipliez cela par 8). Le commentaire aurait dû être:
/* pos is a value between [0, BIT) */ Notez lutilisation de « [ » à gauche et « ) » à droite. Il sagit dune notation mathématique indiquant que 0 est inclusif et que BIT nest pas inclus dans la plage (cest courant dans la documentation C / C ++).
Réponse
-
Vos types sont inutiles – utilisez simplement char et int non signés comme le ferait la bibliothèque standard.
-
Sur le même sujet, votre enum bool est inutile car en C non nul indique le vrai état. Vous le confirmez vous-même car get () suppose que la valeur 1 est vraie. Tout le monde suppose cela et donc le type est redondant. De plus, cest également faux car si je change votre énumération en
typedef enum{true=0, false} bool;votre fonction échoue, même si cest logiquement une chose raisonnable à faire.
Si vous voulez utiliser un booléen comme celui-là, vous devez renvoyer une de ses valeurs explicitement:
return ((a >> pos) & 1) ? true : false;mais cela ne sert à rien. Renvoyez simplement un entier comme le ferait la bibliothèque standard.
-
bitmapGet () renvoyant un bool me semble mal nommé. Les bits bitmap ont les valeurs 1 et 0, pas vrai et faux (même si ce sont actuellement les mêmes). Lappeler bitmapIsSet () serait plus logique.
-
sizeof (char) est 1 par définition, donc BIT peut être remplacé par CHAR_BIT
-
Les crochets ouvrants pour les fonctions se trouvent normalement à la colonne 0
-
Lordre des paramètres de bitmapSearch serait plus logique étant la taille suivie de lélément auquel il fait référence (bitmap).
-
Le paramètre de début de bitmapSearch est incorrect. Pour spécifier une recherche à partir du bit 0 (sûrement le plus courant) lappelant doit passer -1 !!
-
Pourquoi passer le paramètre « pos » comme un « octet »? Vous avez défini « byte » pour représenter les octets bitmap et « pos » nen fait certainement pas partie. Vous obtiendrez des avertissements du compilateur sils sont activés (ils devraient lêtre) à propos du passage darguments avec des largeurs différentes en raison de prototypes. Et restreindre « pos » à un octet peut ajouter une instruction machine supplémentaire (regardez lassembleur) tout en ne réalisant rien. Si vous pouviez définir un type pos_t avec la plage 0..7 que vous semblez vouloir, vous pourriez dire quil était en quelque sorte correct, mais comme vous ne pouvez pas le faire en C et clairement la valeur doctet a la plage 0 .. 255, ce nest pas mieux que dutiliser un int.
Answer
Do didn « t poser des questions sur les performances, mais vous pouvez accélérer bitmapSearch pour les bitmaps épars en examinant dabord un octet à la fois. Si vous recherchez un 1, vous pouvez ignorer les octets qui sont 0x00, et si à la recherche dun 0, vous pouvez ignorer les octets qui sont 0xFF. Après cela, vous pouvez analyser les bits sur loctet suivant ou utiliser une table de recherche dessus.
En ce qui concerne lAPI, vous pouvez ajouter
byte * bitmapAlloc(int sizeInBits); void bitmapFree(byte * bitmap); pour que le client nait pas à se soucier du nombre doctets à allocate.
Et / ou fournissez une macro pour aider avec les allocations. Une macro au lieu dune fonction si vous voulez quelle apparaisse dans les allocations de tableau.
#define BITMAP_BYTE_COUNT_FOR(n) ((n)+BIT-1)/BIT) Jai aussi bitmapSearch prendre sa taille comme un nombre de bits au lieu dun nombre doctets. Ensuite, si le client ne se soucie que de, disons, 26 bits pour les lettres, il na pas à se soucier dune recherche renvoyant 27 parce que le bitmap contient en réalité 32 bits.