asked on

What is wrong with this function?

I have the following function. It executes the 'else' clause even when the 'if' clause is executed. I cannot figure out what is going on (I'm assuming I'm missing something really easy, but I can't see it).

      //make the e-mail lower case and remove invalid chars
      string::iterator i;
      string::iterator ib = email->begin();
      string::iterator ie = email->end();

      if(email->length() > 0)
      {
            for ( i = ib; i != ie; i++) {
                  char c = *i;
                  if ( !isalnum(c) && '_' != c && '@' != c && '.' != c) {
                        i = email->erase(i);
                        i--;
                  }
                  else
                  {
                        *i = tolower(c);
                  }
            }
      }

As another question: Since the else clause is always executed, this will cause an infinited low if there are two invalid characters passed in for the email string (e.g. "!!"). Erase will move the string::iterator forward to the next character, and then *i = tolower(c) will keep assigning this character to the previous invalid character that is stored in c. So, if you are at the end of the string, it should loop forever, becuase you keep assigned the end of the string to tolower(c). Is it valid to assign the end of a string to a character like that? I wouldn't think it would be.

If it is, does it just append this to the end of the string?

Thanks ahead of time.

ASKER CERTIFIED SOLUTION

mrwad99

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

mrwad99

Overall this works fine for me:

      std::string* email = new string("D%%avId_J&^|\@mYWebSi*t&^e#!#$%*().co!$%m");

      string::iterator i;
      string::iterator ib = email->begin();
      string::iterator ie;


      if(email->length() > 0)
      {
            for ( i = ib; i != email->end(); i++) {
                  char c = *i;
                  if ( !isalnum(c) && (c != '_') && (c != '@') && (c != '.') ) {
                        i = email->erase(i);
                        i--;
                  }
                  else
                  {
                        *i = tolower(c);
                  }
            }
      }
      cout << *email;

>> As another question: Since the else clause is always executed...

I don't get the else executed always, only if the if fails.

HTH

jkr

mrwad99

>>i = email->erase(i);
>>will invalidate the iterator ie if this is executed

That's why the return value of 'erase()' is used. That one *is* valid.

jjacksn

ASKER

the always executing problem is something bad with my debugger/source code i think, I adding in Message Boxes and they aren't getting displayed. As to the change to the for loop, that worked.

mrwad99

jkr,

Yeah that is correct, the return value of erase is being assigned to i, but when the for loop in the original code is then re-executed, the test will still compare i to ie. At this point ie is of course invalid.

Please correct me if needed; as I have always said - your record talks for itself...