2009-04-01 7 views
2

Dieser Code funktioniert, aber ich frage mich, ob es einen besseren Weg gibt, es zu tun. Grundsätzlich muss ich Bits testen und schreibe das passende Zeichen oder Zeichen in eine Zeichenkette, abhängig vom Zustand des Bits. Die Leerzeichen sind vorhanden, weil die Zeichen mit einer Schriftart mit fester Breite angezeigt werden, und ich möchte sie davon abhalten, sich zu bewegen. C oder C++ ist in Ordnung.Testen von Bits zum Erstellen eines Strings - Gibt es einen besseren Ansatz?

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

Antwort

4
  • Verwendung std :: string statt char * und strcat;
  • Warum brauchen Sie Array mit Leerzeichen? es könnte nur ein Raum sein;
  • Sie haben fast identische Code für zwei U16-Parameter - machen Sie eine kleine Funktion und rufen Sie zweimal;
  • schreiben in globalen Variablen nicht zur Folge haben - zurück std :: string
+0

Es macht Sie sicher, dass die Breite der gleiche ist, ob ein Bit gesetzt ist oder nicht. – MSN

1

Auf Kosten einiger dynamische Zuweisungen (innen std :: string), können Sie diesen Code leichter modifizierbar machen, indem nicht mit fest codierte Zahlen:

#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

Das sollte in Ordnung sein; Wenn Sie jedoch Schaltflächen oder Achsen hinzufügen möchten, möchten Sie es vielleicht etwas verallgemeinern.

3

Ich empfehle etwas etwas expliziter, die keine Schleifen verwendet, weil Sie nur eine kleine Anzahl von Bits zu überprüfen scheinen. Wenn dies auf Zehntausende von Bits skaliert werden muss, dann verwenden Sie unbedingt Schleifen.

Ich gehe auch davon aus, dass Sie einen guten Grund haben, globale Variablen und Zeichenfelder fester Länge zu verwenden. Hier

ist, was ich tun würde:

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

Grundsätzlich ist die C++ Lösung sieht aus wie

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

Wo Converter wie

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

Hier ist die convert von Codes zu Zeichenkettenfunktion

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

Ich denke, es wird besser sein, boost :: array oder üblichen Array für Store-Strings (einfach mit der Erstellung) zu verwenden. Auch dein Converter zählt Anrufe - ich bin mir nicht sicher, ob das legal ist. – bayda

+0

@bb: natürlich kann die Initialisierungsphase geändert werden oder sogar Konverter kann Sammlungen als Parameter akzeptieren. boost :: array ist eine gute Idee, falls Boost erlaubt ist. Benutzt gewöhnliche Arrays nur für die Kürze - ich mag sie nicht :) und ihre Größenberechnung. Einchecken über Fünfer. –

+0

Welchen Vorteil hat das Schreiben von Code, der kompliziert ist? – alexk7

0

Hier ist eine bit-twiddly Art, es in einem einzigen Durchgang zu tun. Es ist sogar erweiterbar auf bis zu 16 Bits, solange Sie sicherstellen, dass die wide Maske ein Bit gesetzt hat, wo immer Sie einen 2-stelligen Signifier haben.

#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

Nicht Hacky, saubere Lösung:

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