• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 191
  • Last Modified:

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.
0
jjacksn
Asked:
jjacksn
  • 3
1 Solution
 
mrwad99Commented:
I think what you need to do is alter the for loop

for ( i = ib; i != email->end(); i++)

since

i = email->erase(i);

will invalidate the iterator ie if this is executed.  Never store the value of an iterator if it could be later changed.

Try that.
0
 
mrwad99Commented:
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
0
 
jkrCommented:
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.
0
 
jjacksnAuthor Commented:
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.
0
 
mrwad99Commented:
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...
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 3
Tackle projects and never again get stuck behind a technical roadblock.
Join Now