Home > database >  How can I compact this code? Too many if statements
How can I compact this code? Too many if statements

Time:09-16

Is there any way to compact this code I have? This is based on ASCII values and I'm trying to create a table for it. I can create a map which would probably be WAY simpler but I don't know if my professor is allowing that for some reason.

If I did do a map I'd just create one with all the standard ascii values rather than having all of these statements. But for the sake of this method I thought about implementing && and ! and || but dont know where to start.

void mark_cells(int row, int _table[][MAX_COLUMNS], const char columns[], int state) {
    if (columns == ALFA) {
        // A-Z
        for (int i = 65; i <= 90;   i) {
            _table[row][i] = state;
        }
        // a-z
        for (int i = 97; i <= 122;   i) {
            _table[row][i] = state;
        }
    }
    else if (columns == DIGITS) {
        // 0 - 9
        for (int i = 48; i <= 57;   i) {
            _table[row][i] = state;
        }
    }
    else if (columns == OPERATORS) {
        // <, =, >
        for (int i = 60; i <= 62;   i) {
            _table[row][i] = state;
        }
        // (,),*, 
        for (int i = 40; i <= 43;   i) {
            _table[row][i] = state;
        }
        // {,|,}
        for (int i = 123; i <= 125;   i) {
            _table[row][i] = state;
        }
        // !
        _table[row][33] = state;
        // %
        _table[row][37] = state;
        // &
        _table[row][38] = state;
        // -
        _table[row][45] = state;
    }
    else if (columns == SPACES) {
        _table[row][32] = state;
    }
    else if (columns == PUNC) {
        // ;
        _table[row][58] = state;
        // :
        _table[row][59] = state;
        // ?
        _table[row][63] = state;
        // '
        _table[row][39] = state;
        // .
        _table[row][46] = state;
        // ~
        _table[row][126] = state;
        // !
        _table[row][33] = state;
        // `
        _table[row][96] = state;
        // ,
        _table[row][44] = state;
        // -
        _table[row][45] = state;
    }
    
}

CodePudding user response:

First, I don't know how this is supposed to work:

if (columns == ALFA)

If columns is of type char[]. Is that a pointer comparison or a string comparison? If the latter, it doesn't do what you think it will do (reliably).

If it's a string comparison, change column to be of type const std::string&. Then those comparison to defined literals will work better.

For starters, have a helper function that does that loop that keeps repeating.

void setStateOnRange(int table[][MAX_COLUMNS], int row, int start, int end, int state) {
        for (int i = start; i <= end;   i) {
            table[row][i] = state;
        }
}

Then your function becomes a lot easier to write:

void mark_cells(int row, int _table[][MAX_COLUMNS], const string& columns, int state) {
    if (columns == ALFA) {
        // A-Z
        setStateOnRange(_table, row, 'A','Z');
        setStateOnRange(_table, row, a','z');
    }
    else if (columns == DIGITS) {
        // 0 - 9
        setStateOnRange(_table, row, '0','9');
    }
    ...

Another idea is to have a static table that maps each ascii value to the column range you expect to have. I'll let you intialize the table accordingly. The implementation becomes really short.

void mark_cells(int row, int _table[][MAX_COLUMNS], const string& columns, int state) {

    static string columnmap[128] = {
           /* 0..31*/ NONE, NONE, NONE, ... <32 NONES in a row>,
           /* 32-47*/ SPACE, PUNC, PUNC, PUNC....
           /*48-57*/  DIGIT, DIGIT, DIGIT.... 
                      PUNC, PUNC, PUNC...
           /*65-90*/  ALFA, ALFA, etc....
           ...};

     for (int i = 0; i < 128; i  ) {
         if (columnmap[i] == columns) {
             table[row][i] = state;
         }
     }
 }

CodePudding user response:

I would just begin by making each group of characters a string. For example:

const char* allAlpha = "QWERTYUIOPASDFGHJKLZXCVBNM"
                       "qwertyuiopasdfghjklzxcvbnm";
const char* allDigits = "1234567890";
const char* allOperators = "<=>,* !%&-";
...

Likewise for the others.. Then your logic would select one:

const char* selected = nullptr;
if (columns == ALFA)
    selected = allAlpha;
else if (columns == DIGITS)
    selected = allDigits;
else if (columns == OPERATORS)
    selected = allOperators;
...

Finally, if you selected something you can can walk through it to mark all these states:

if (selected)
{
    for (const char* p = selected; *p != '\0';   p)
        _table[row][(unsigned char)*p] = state;
}
  • Related