2009-04-01 6 views
2

Ce code fonctionne, mais je me demande s'il existe une meilleure façon de le faire. Fondamentalement, j'ai besoin de tester les bits, et écrire le ou les caractères appropriés à une chaîne en fonction de l'état du bit. Les espaces sont présents parce que les caractères seront affichés avec une police de largeur fixe et je voudrais les empêcher de se déplacer. C ou C++ est bien.Test de bits pour créer une chaîne - Y a-t-il une meilleure approche?

const char* Letters[10] = {"A", "B", "Sl", "St", "R", "L", "U", "D", "RS", "LS"}; 
const char* Ext[2] = {"X", "Y"}; 
const char* Spaces[10] = {" ", " ", " ", " ", " ", " ", " ", " ", " ", " "}; 

char str[60]; 
char FinalString[60]; 

void MakeBitString(u16 data, u16 dataExt) { 

    int x; 
    strcpy(str, ""); 

    for (x = 0; x < 2; x++) { 

     //X and Y 
     if(dataExt & (1 << x)) { 
      strcat(str, Spaces[x]); 
     } 
     else 
      strcat(str, Ext[x]); 
    } 

    for (x = 0; x < 10; x++) { 

     //the rest 
     if(data & (1 << x)) { 
      strcat(str, Spaces[x]); 
     } 
     else 
      strcat(str, Letters[x]); 
    } 

    strcpy(FinalString, str); 
} 

Répondre

4
  • utilisation std :: string au lieu char * et strcat; Pourquoi avez-vous besoin d'un tableau avec des espaces? il semble que ce pourrait être juste un espace;
  • vous avez presque le code indentical pour deux paramètres u16 - faites une petite fonction et appelez-le deux fois;
  • ne pas écrire résultat dans la variable globale - retour std :: string
+0

C'est en vous assurant que la largeur est la même si un bit est réglé ou non. – MSN

1

Au prix de certaines allocations dynamiques (à l'intérieur std :: string), vous pouvez rendre ce code plus facilement modifiable par ne pas avoir de numéros codés en dur:

#define ARRAYSIZE(A) (sizeof(A)/sizeof((A)[0])) 

std::string MakeBitString(u16 data, const std::string* letters, int count) { 
    std::string s; 
    for (int x = 0; x < count; x++) { 
     if (data & (1 << x)) 
      s.append(letters[x].size(), ' '); 
     else 
      s += letters[x]; 
    } 
    return s; 
} 

std::string MakeBitString(u16 data, u16 dataExt) { 
    const std::string Letters[] = {"A", "B", "Sl", "St", "R", "L", "U", "D", "RS", "LS"}; 
    const std::string Ext[] = {"X", "Y"}; 

    std::string s = MakeBitString(dataExt, Ext, ARRAYSIZE(Ext)); 
    s += MakeBitString(dataExt, Letters, ARRAYSIZE(Letters)); 
    return s; 
} 
0

Cela devrait être bon; Toutefois, si vous voulez ajouter des boutons ou des axes, vous pouvez le généraliser un peu.

3

Je recommande quelque chose d'un peu plus explicite, qui n'utilise pas de boucles, parce que vous semblez n'avoir qu'un petit nombre de bits à vérifier. Si cela doit atteindre des dizaines de milliers de bits, alors utilisez des boucles.

Je suppose également que vous avez une bonne raison d'utiliser des variables globales et des tableaux de caractères de longueur fixe.

Voici ce que je ferais:

char FinalString[60]; 

void ConcatBitLabel(char ** str, u16 data, u16 bitMask, const char * label) 
{ 
    if (data & bitMask) 
    { 
     // append spaces for strlen(label) 
     while (*label) { *((*str)++) = ' '; label++; } 
    } 
    else 
    { 
     // append the label 
     while (*label) { *((*str)++) = *label; label++; } 
    } 
} 

void MakeBitString(u16 data, u16 dataExt) 
{ 
    char * strPtr = FinalString; 

    ConcatBitLabel(&strPtr, dataExt, 0x0001, "X"); 
    ConcatBitLabel(&strPtr, dataExt, 0x0002, "Y"); 

    ConcatBitLabel(&strPtr, data, 0x0001, "A"); 
    ConcatBitLabel(&strPtr, data, 0x0002, "B"); 
    ConcatBitLabel(&strPtr, data, 0x0004, "Sl"); 
    ConcatBitLabel(&strPtr, data, 0x0008, "St"); 
    ConcatBitLabel(&strPtr, data, 0x0010, "R"); 
    ConcatBitLabel(&strPtr, data, 0x0020, "L"); 
    ConcatBitLabel(&strPtr, data, 0x0040, "U"); 
    ConcatBitLabel(&strPtr, data, 0x0080, "D"); 
    ConcatBitLabel(&strPtr, data, 0x0100, "RS"); 
    ConcatBitLabel(&strPtr, data, 0x0200, "LS"); 

    *strPtr = 0; // terminate the string 
} 
3

Fondamentalement, la solution C++ ressemble

Codes convert(std::size_t data, 
       const Codes& ext, 
       const Codes& letters) 
{ 
    Codes result; 
    std::transform(ext.begin(), 
        ext.end(), 
        std::back_inserter(result), 
        Converter(data)); 

    std::transform(letters.begin(), 
        letters.end(), 
        std::back_inserter(result), 
        Converter(data)); 
    return result; 
} 

Converter est mis en œuvre comme

struct Converter 
{ 
    Converter(std::size_t value): 
     value_(value), x_(0) 
    {} 
    std::string operator() (const std::string& bitPresentation) 
    { 
     return (value_ & (1 << x_++)) ? 
      std::string(bitPresentation.size(), ' '): 
      bitPresentation; 
    } 
    std::size_t value_; 
    std::size_t x_; 
}; 

Voici le converti de codes à fonction de chaîne

std::string codesToString(const Codes& codes) 
{ 
    std::ostringstream stringStream; 
    std::copy(codes.begin(), codes.end(), 
       std::ostream_iterator<std::string>(stringStream)); 
    return stringStream.str(); 
} 
+0

Je pense qu'il vaudra mieux utiliser boost :: array ou aray habituel pour les chaînes de stockage (facile avec la création). Aussi votre convertisseur compte les appels - je ne suis pas sûr que ce soit légal. – bayda

+0

@bb: bien sûr, la phase d'initialisation peut être modifiée ou même le convertisseur peut accepter les collections en tant que paramètres. boost :: array est une bonne idée dans le cas où boost est autorisé. En utilisant des tableaux habituels juste pour raccourcir - je ne les aime pas :) et leur calcul de taille. Avoir à vérifier en standard sur le comptage dans functor. –

+0

Quel est l'avantage d'écrire du code compliqué? – alexk7

0

Voici une façon un peu brusque de le faire en un seul passage. Il est même extensible jusqu'à 16 bits, à condition que vous vous assuriez que le masque wide est un peu défini partout où vous avez un signifiant à 2 caractères.

#define EXT_STR "XY" 
#define DATA_STR "ABSlStRLUDRSLS" 
const char FullStr[] = EXT_STR DATA_STR; 
#define EXT_SZ 2 //strlen(EXT_STR); 

void MakeBitStr(u16 data, u16 dataExt) { 
    char* dest = FinalString; 
    const char* src= FullStr; 
    u16 input = (data<<EXT_SZ)|dataExt; 
    u16 wide = (0x30C<<EXT_SZ)|0; //set bit for every 2char tag; 
    while ((src-FullStr)<sizeof(FullStr)) 
    { *dest++ = (input&1)?' ':*src; 
     if (wide&1) 
     { wide&=~1; 
     } 
     else 
     { input>>=1;wide>>=1; 
     } 
     src++; 
    } 
    *dest='\0'; 
} 
0

solution non-aki, propre:

std::string MakeBitString(u16 data, u16 dataExt) { 
    std::string ret; 

    static const char *letters = "A B SlStR L U D RSLS"; 
    static const char *ext = "XY"; 
    static const char *spaces = " "; 

    for(int bit = 0; bit < 2; ++bit) { 
     const char *which = (dataExt & 1) ? &ext[bit] : spaces; 

     ret += std::string(which, 0, 1); 

     dataExt >>= 1; 
    } 

    for(int bit = 0; bit < 10; ++bit) { 
     const int length = letters[bit * 2 + 1] == ' ' ? 1 : 2; 
     const char *which = (dataExt & 1) ? &letters[bit * 2] : spaces; 

     ret += std::string(which, 0, length); 

     dataExt >>= 1; 
    } 

    return ret; 
} 
Questions connexes