CODEWITHSUNDEEP
c++performanceswitch-statement
Tyler Pfaff
Tyler Pfaff

Reputation: 5032

How to avoid long switch statements? C++

I am working on a "dictionary" for my class. I have an int array called NumOfWordsInFile[] where NumOfWordsInFile[0] corresponds to how many words are in A.txt and NumOfWordsInFile[25] corresponds to Z.txt

As it is now I have a huge switch for the 26 different conditions of letters. I have a function called AddWord(string word). AddWord gets the first letter of the word passed to it and inserts it into the appropriate .txt file. Now here is the problem. Everytime a word is added to A.txt I must increment NumOfWordsInFile[0] by 1. The only way I can think of to do this is with these huge switches. I also have a deleteWord function which conversely decrements NumOfWordsInFile[] if the word is deleted. Now I dont want to have two 26 case swithes but the problem is I dont know how else to do it. Now I could just do the same thing for the delete function but I really dont want to have hundreds of more lines of code to go through. Is there a better way to do this?

Sample of the switch in the AddWord function:

case 'w':
    if (numOfWordsInFile[22] < maxWordsPerFile) {
        fout.open(fileName.data(), ios::app);
        fout << word << " " << endl;
        numOfWordsInFile[22]++;
        if (totalWordsInDict < maxWordsInDict) {
            totalWordsInDict++;
        }
        return(Dictionary::success);
    } else {
        return(Dictionary::failure);
    }

case 'x':
    if (numOfWordsInFile[23] < maxWordsPerFile) {
        fout.open(fileName.data(),ios::app);
        fout << word << " " << endl;
        numOfWordsInFile[23]++;
        if (totalWordsInDict < maxWordsInDict) {
            totalWordsInDict++;
        }
        return(Dictionary::success);
    } else {
        return(Dictionary::failure);
    }

Delete function.

bool Dictionary::DeleteAWord(string word)
{
    ofstream fout;
    ifstream fin;
    string x;
    string fileName="#.txt";
    int count=0;
    vector <string> words;
    bool deleted=false;

    fileName[0]=toupper(word[0]);
    fin.open(fileName.data()); //makes the file depending on the first letter of the argument "word"

    while (fin >> x)
    {
        words.push_back(x);
        count++;//number of elements in vector
    }
    if (SearchForWord(x))
    {
        for ( ;count > 0; count--)
        {
            if (words[count-1] == word)
            {
                // cout << "Found word " << word << " during search, now deleting" << endl;
                words.erase(words.begin()+(count-1));
                deleted = true;

                /*
                    This clearly doesn't work and is what I need help with, I know why it
                    doesn't work but I don't know how to make it better than having another
                    huge switch.
                */
                numOfWordsInFile[toupper(word[0])]--;
                /*

                */

                totalWordsInDict--;
                fin.close();
            }
        }

        if (deleted)
        {
            fout.open(fileName.data());
            for (int i = 0; i < words.size(); i++)
                fout << words[i] << endl;
            return(Dictionary::success);
        }
        return(Dictionary::failure);
    }
    return(Dictionary::failure);
}

Upvotes: 8

Views: 2760

Answers (8)

Oliver Charlesworth
Oliver Charlesworth

Reputation: 272607

In most practical character encodings that you're likely to encounter whilst using C or C++, 'a' to 'z' are contiguous, so you can get the array index to use simply by doing (c - 'a'), where c is the char you're looking at.

Upvotes: 6

James Kanze
James Kanze

Reputation: 153957

It depends on how portable you want to be, or how internationalized. If you can afford to ignore the possibility that the first letter might be an accented character, and assume that you're never going to have run on a mainframe, or anywhere else that uses EBCDIC, then you can convert the first letter to a specific case, and subtract 'a' or 'A' (depending on the case) from it to obtain the index. The C++ standard doesn't guarantee that the letters are contiguous however, and they aren't in EBCDIC, nor in any of the encodings which support accented characters. At the very least, you'll have to test that the first character is a letter, of course.

Handling the internationalization issue is difficult, since there is no one generally used encoding, and some of the encodings are multibyte. For the single byte encodings, it's fairly straight foreward to use a mapping table; a table with 256 entries, indexed by the first letter (cast to unsigned char), which returns the index into your table. For multibyte encodings, like UTF-8, the issue is more complicated: you can translate the initial character in a UTF-8 sequence to an int, but you can end up with values around a million or more, and you don't want a table with a million entries (most of which are completely irrelevant. One simple solution might be to add a 27th entry for "other". (This would also catch "words" like "2nd".)

A very portable way to do this would be:

int mappingTable[256];

std::fill_n(mappingTable, 256, 26);
static char const upper[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ;
static char const lower[] = "abcdefghijklmnopqrstuvwxyz;
for (int i = 0; i < 26; ++ i) {
    mappingTable[upper[i]] = i;
    mappingTable[lower[i]] = i;
}

Just don't forget to cast the initial character to unsigned char before indexing.

Upvotes: 1

Mike Bailey
Mike Bailey

Reputation: 12817

Just taking a very quick look, it seems like you're using the position of the letter in the alphabet to do stuff.

You could replace all your switch statements with one statement that looks like:

int letter = (int)(ActualLetter - 'a');

if(numOfWordsInFile[letter]<maxWordsPerFile){
 fout.open(fileName.data(),ios::app);
 fout<<word<<" "<<endl;
 numOfWordsInFile[letter]++;
 if(totalWordsInDict<maxWordsInDict){
   totalWordsInDict++;
 }
 return(Dictionary::success);
}else{
 return(Dictionary::failure);
}

ActualLetter is something like, 'a', for example.

On a related note, in the future if you actually have large switch statements, consider encapsulating the code in functions:

switch (letter)
{
    case 'a':
      LetterA();
      break;

    case 'b':
      LetterB();
      break;

    ...
}

Or even better, you can use polymorphism to have C++ dispatch to the method you want based on the specific derived class:

class BaseLetter
{
   ...
public:
   virtual void DoStuff() = 0;
};

class LetterA : public BaseLetter
{
public:
   void DoStuff();
};

class LetterB : public BaseLetter
{
public:
    void DoStuff();
};

void Foo(BaseLetter *letter)
{
    // Use dynamic dispatch to figure out what to do
    letter->DoStuff();
}

Just note, dynamic dispatch does have a (slight) performance hit, and the above is a very bad place to actually use it. The solution I, RedX, and others have posted is much better suited to your specific example.

Upvotes: 7

Dariusz Jędrzejczyk
Dariusz Jędrzejczyk

Reputation: 911

Chars are basically numbers. 'a' is 97, 'b' is 98 and so on. The easiest way is to simply replace every numOfWordsInFile[n] with numOfWordsInFile[current_char - 'a'] and the whole code repeated for each case may reside in a function, like this:

   int AddWord(char current_char) {
    if(numOfWordsInFile[current_char - 'a']<maxWordsPerFile){
     fout.open(fileName.data(),ios::app);
     fout<<word<<" "<<endl;
     numOfWordsInFile[current_char - 'a']++;
      if(totalWordsInDict<maxWordsInDict){
       totalWordsInDict++;
     }
     return(Dictionary::success);
    }else{
     return(Dictionary::failure);
    }
   }

For more general solutions read about hash maps and function pointers (when, for instance, for each char you might want to assign a different function.

Upvotes: 3

jonsca
jonsca

Reputation: 10381

If your file is A.txt, let your array index be 'A' - 'A' (= 0), if the file is B.txt, let the array index be 'B' - 'A' (= 1), etc.

Upvotes: 1

Erik
Erik

Reputation: 91280

struct FileInfo {
  int NumWords;
  std::string Filename;
};

std::map<char, FileInfo> TheFiles; 

FileInfo & FI = TheFiles[letter];
// Work with FI.NumWords and FI.Filename

Alternatively:

std::vector<FileInfo> TheFiles;
FileInfo & FI = TheFiles[std::tolower(Letter) - 'a'];

Upvotes: 6

SoapBox
SoapBox

Reputation: 20609

Single characters in C++ are really just numbers corresponding to their ASCII values. You can subtract letters from each other to get numerical values. So if word[0] contains the letter A, then word[0] - 'A' will be 0.

So you can index your numOfWordsInFile array directly, and you won't need a switch at all: numOfWordsInFiled[word[0] - 'A'].

Note that 'A' and 'a' have different numeric values, so you'll have to do some extra work if you're mixing upper and lower case.

Upvotes: 2

RedX
RedX

Reputation: 15175

if(numOfWordsInFile[letter - 'A']<maxWordsPerFile){
 fout.open(fileName.data(),ios::app);
 fout<<word<<" "<<endl;
 numOfWordsInFile[letter - 'A']++;
 if(totalWordsInDict<maxWordsInDict){
   totalWordsInDict++;
 }
 return(Dictionary::success);
}else{
 return(Dictionary::failure);
}

This will only work if you only have english letter in your use-case.

Upvotes: 3

Related Questions