Ik ben onlangs begonnen met het opschonen van mijn projecten en besloot dat ik een persoonlijke opslagplaats met nuttige stukjes code wilde maken. Ik vond deze bitmapcode tussen hen in en heb deze zwaar bewerkt en ik zou het op prijs stellen als iemand het zou kunnen beoordelen, want ik ben niet zo gewend aan bitgewijze bewerkingen en dergelijke. Is de code draagbaar genoeg, of zouden er wijzigingen moeten zijn?
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); } Opmerkingen
- Aan iedereen die dit bericht nog steeds volgt, heb ik de code toegewezen aan hier . Ik heb ook al uw opmerkingen als problemen toegevoegd. Bedankt voor uw opmerkingen en ik zal beginnen met de implementatie ervan.
Antwoord
Als u verwacht dat uw code ooit zal zijn gebruikt vanuit C ++ (wat veel C-code is), gebruik dan geen identifiers met een dubbel onderstrepingsteken.
#ifndef BITMAP__ // ^^ May cause problems in C++ Aangezien dit is gereserveerd voor de implementatie in C ++. De term BITMAP is ook erg algemeen en het is zeer waarschijnlijk dat je botst met andere bibliotheken, probeer het unieker te maken voor je project.
Je gaat ervan uit dat een teken 8 bits is.
#define BIT (8*sizeof(byte)) De standaard zegt dit niet (het is minstens 8 bits) maar het is eigenlijk gedefinieerd via de macro CHAR_BITS, dus je zou moeten gebruiken:
#define BIT (CHAR_BITS*sizeof(byte)) Uw header-bestand is hoe de meeste mensen eerst uw code zullen zien en zullen proberen te begrijpen hoe ze deze moeten gebruiken. Het is dus waarschijnlijk een goed idee om de parameternamen in het headerbestand te plaatsen, omdat het helpt bij het documenteren van hun gebruik.
bool bitmapGet (byte *, int); void bitmapSet (byte *, int); void bitmapReset (byte *, int); int bitmapSearch(byte *, bool, int, int); Aanvullend op de actie van bitmapSearch ligt niet voor de hand, dus een opmerking over het gebruik ervan in de koptekst is waarschijnlijk een goed idee. Eigenlijk zou een kleine blurb over hoe de eerste parameter een array kan zijn waarschijnlijk een goed idee zijn (aangezien het niet duidelijk is zonder de code te lezen).
Wees voorzichtig met opmerkingen:
/* pos is something from 0 to 7*/ Dit is alleen waar als je aannames doet (A: die grootte van (byte) == 1 B: je vermenigvuldigt dit met 8). De opmerking had moeten zijn:
/* pos is a value between [0, BIT) */ Let op het gebruik van “[” aan de linkerkant en “)” aan de rechterkant. Het is een wiskundige notatie die aangeeft dat 0 inclusief is en BIT niet in het bereik is opgenomen (het is gebruikelijk in zowel C / C ++ documentatie).
Antwoord
-
Uw typen zijn niet nodig – gebruik gewoon unsigned char en int zoals de standaardbibliotheek dat zou doen.
-
Over hetzelfde onderwerp is uw enum bool niet nodig omdat in C niet-nul de ware toestand aangeeft. Dat bevestig je zelf, want get () gaat ervan uit dat de waarde 1 waar is. Iedereen gaat ervan uit en dus is het type overbodig. Bovendien is het ook verkeerd, want als ik je enum verander in
typedef enum{true=0, false} bool;, faalt je functie, ook al is het logischerwijs redelijk om te doen.
Als je zon bool wilt gebruiken, zou je een van zijn waarden expliciet moeten teruggeven:
return ((a >> pos) & 1) ? true : false;maar dit heeft echt geen zin. Retourneer gewoon een int zoals de standaardbibliotheek dat zou doen.
-
bitmapGet () die een bool retourneert, lijkt mij een verkeerde naam. De bitmapbits hebben de waarden 1 en 0, niet true en false (hoewel deze momenteel hetzelfde zijn). Het bitmapIsSet () noemen zou logischer zijn.
-
sizeof (char) is per definitie 1, dus BIT kan worden vervangen door CHAR_BIT
-
Haakjes openen voor functies staan normaal gesproken in kolom 0
-
De volgorde van de parameters voor bitmapSearch zou logischer zijn, is de grootte die volgt op het item waarnaar het verwijst (bitmap).
-
De startparameter voor bitmapSearch is verkeerd. Om een zoekopdracht te specificeren beginnend bij bit 0 (zeker de meest voorkomende), moet de beller -1 doorgeven !!
-
Waarom de “pos” parameter doorgeven als een “byte”? U hebt “byte” gedefinieerd om de bitmapbytes weer te geven en “pos” is daar zeker niet een van. Je krijgt compilerwaarschuwingen als ze zijn ingeschakeld (dat zouden ze moeten zijn) over het doorgeven van argumenten met verschillende breedtes vanwege prototypes. En het beperken van “pos” tot een byte kan een extra machine-instructie toevoegen (kijk naar de assembler) terwijl je niets bereikt. Als je een type pos_t zou kunnen definiëren met het bereik 0..7 dat je lijkt te willen, dan zou je kunnen zeggen dat het op de een of andere manier correct was, maar aangezien je dat niet kunt doen in C en de bytewaarde heeft duidelijk het bereik 0 .. 255, is het niet beter dan een int te gebruiken.
Antwoord
Heb niet “t vraag naar prestaties, maar u kunt bitmapSearch versnellen voor schaarse bitmaps door eerst naar byte tegelijk te kijken. Als u naar een 1 zoekt, kunt u bytes overslaan die 0x00 zijn, en als zoekend naar een 0, kun je bytes overslaan die 0xFF zijn. Daarna kun je de bits op de volgende byte scannen of er een opzoektabel op gebruiken.
Wat de API betreft, kunt u
byte * bitmapAlloc(int sizeInBits); void bitmapFree(byte * bitmap); toevoegen zodat de client zich geen zorgen hoeft te maken over hoeveel bytes allocate.
En / of bied een macro om te helpen met allocaties. Een macro in plaats van een functie als je wilt dat deze verschijnt in array allocaties.
#define BITMAP_BYTE_COUNT_FOR(n) ((n)+BIT-1)/BIT) Ik “d had ook bitmapSearch de grootte ervan als een bit-telling in plaats van een bytetelling. Als de cliënt dan bijvoorbeeld maar om 26 bits geeft voor letters, hoeven ze zich geen zorgen te maken dat een zoekopdracht 27 oplevert, omdat de bitmap echt 32 bits bevat.