Function for removing forbidden characters

Question

void removeForbiddenChar(string* s)
{
    string::iterator it;

    for (it = s->begin() ; it < s->end() ; ++it){
        switch(*it){
        case '/':case '\\':case ':':case '?':case '"':case '<':case '>':case '|':
            *it = ' ';
        }
    }
}

I used this function to remove a string that has any of the following character: \, /, :, ?, ", <, >, |. This is for a file's name. This program runs fine. It simply change a character of the string to a blank when the respective character is the forbidden character. However, I have a feeling against this use of switch statement. I simply exploit the case syntax here, but this, somehow nags me. I just don't like it. Anybody else got a better suggestion of a better implementation in this case?

Answer 1

Declare a string containing the illegal characters: "\\/:?"<>|". All you need to do is check if the char is in the array, so use a native function for that, or write a method CharInString(char* needle, string* haystack) which loops through the contents of the provided haystack to check if the needle is inside it.

Your loop should end up looking like this:

string illegalChars = "\\/:?\"<>|"
for (it = s->begin() ; it < s->end() ; ++it){
    bool found = illegalChars.find(*it) != string::npos;
    if(found){
        *it = ' ';
    }
}

It's more maintainable and readable. You can tell if you've duplicated a character quite easily and since you can do it with any target string and any string of illegalChars you've just created for yourself a generic RemoveIllegalChars(string* targetString, string* illegalChars) method usable anywhere in your program.

_{I may be using those pointers wrong. My C++fu is weak... for now.}

Answer 2

you could always use transform

#include <algorithm>
#include <string>
#include <iostream>

const std::string forbiddenChars = "\\/:?\"<>|";
static char ClearForbidden(char toCheck)
{
    if(forbiddenChars.find(toCheck) != string::npos)
    {
         return ' ';
    }

    return toCheck;
}

int main()
{
    std::string str = "EXAMPLE:";
    std::transform(str.begin(), str.end(), str.begin(), ClearForbidden);
    std::cout << str << std::endl;
    return 0;
}

Answer 3

Or, here's yet another way you could do it by using all stuff from the STL:

#include <algorithm>
#include <string>
#include <iostream>

bool isForbidden( char c )
{
    static std::string forbiddenChars( "\\/:?\"<>|" );

    return std::string::npos != forbiddenChars.find( c );
}

int main()
{
    std::string myString( "hell?o" );

    std::replace_if( myString.begin(), myString.end(), isForbidden, ' ' );

    std::cout << "Now: " << myString << std::endl;
}

Answer 4

One thing that I would change about your function (in addition to Jonathan's recommendation of using a string to store the forbidden characters), is the argument type of removeForbiddenChar to string& instead of string*. It is generally considered good practice in C++ to use references over pointers where possible (see for example this entry in the C++ faq-lite).

One further, minor cosmetic change I'd recommend is renaming the function to removeForbiddenChars (plural) as that is more descriptive of what it does.

Answer 5

C comes with a helpful function size_t strcspn(const char *string, const char *delimiters) that you can implement this on top of. The ASCII version is pretty fast; it uses a bit vector to test for the delimiter characters.

Answer 6

Solution with no conditional branching.
Swapping space for time optimization.

Simplified algorithm:

void removeForbiddenChar(string* s)
{
    for (string::iterator it = s->begin() ; it < s->end() ; ++it)
    {
        // replace element with their counterpart in the map
        // This replaces forbidden characters with space.
        (*it) = charMap[*it];
    }
}

Or the C++0x version:

void removeForbiddenChar(std::string* s)
{
    std::transform(s->begin(), s->end(), [](char c) => {return charMap[c];});
}

Just need the data:

char    charMap[] =
                            // The majority of characters in this array
                            // map the poistion to the same character code.
                            //  charMap['A']  == 'A'
                            // For forbidden characters a space is in the position
                            //  charMap['<']  == ' '
                            //  Note: \xxx is an octal escape sequence
                            "\000\001\002\003\004\005\006\007"
                            "\010\011\012\013\014\015\016\017"
                            "\020\021\022\023\024\025\026\027"
                            "\030\031\032\033\034\035\036\037"
                            "\040\041 \043\044\045\046\047" // replaced \042(") with space
                            "\050\051\052\053\054\055\056 " // replaced \057(/) with space
                            "\060\061\062\063\064\065\066\067"
                            "\070\071 \073 \075  " // replaced \072(:)\074(<)\076(>)\077(?) with space
                            "\100\101\102\103\104\105\106\107"
                            "\110\111\112\113\114\115\116\117"
                            "\120\121\122\123\124\125\126\127"
                            "\130\131\132\133 \135\136\137" // replaced \134(\)
                            "\140\141\142\143\144\145\146\147"
                            "\150\151\152\153\154\155\156\157"
                            "\160\161\162\163\164\165\166\167"
                            "\170\171\172\173\174\175\176\177"
                            "\200\201\202\203\204\205\206\207"
                            "\210\211\212\213\214\215\216\217"
                            "\220\221\222\223\224\225\226\227"
                            "\230\231\232\233\234\235\236\237"
                            "\240\241\242\243\244\245\246\247"
                            "\250\251\252\253\254\255\256\257"
                            "\260\261\262\263\264\265\266\267"
                            "\270\271\272\273\274\275\276\277"
                            "\300\301\302\303\304\305\306\307"
                            "\310\311\312\313\314\315\316\317"
                            "\320\321\322\323\324\325\326\327"
                            "\330\331\332\333\334\335\336\337"
                            "\340\341\342\343\344\345\346\347"
                            "\350\351\352\353\354\355\356\357"
                            "\360\361\362\363\364\365\366\367"
                            "\370\371\372\373\374\375\376\377";

Answer 7

Similar to strcspn is strpbrk, but instead of returning offsets, it returns a pointer to the next match and NULL if no more matches. This makes the replacement as simple as:

while ((filename = strpbrk(filename , "\\/:?\"<>|")) != NULL)
    *filename++ = '_';