Removing White Spaces

Question

I use the following to remove spaces from my variable

        for (i=0, ptr=lpsz;ptr[i];ptr++)
        {
            if (*ptr == ' ')
                i++;

            *ptr = ptr[i];
        }
        *ptr=0;

It seems to be having problems when there is more than one space however and I am not sure what I am doing wrong. Can anyone help me please?

Answer 1

I suggest you use std::isspace instead of doing your pointer magic.

If you use a std::string in combination with std::isspace you can do something like this:

std::string str = "Hello World Today";

str.erase(remove_if(str.begin(), str.end(), isspace), str.end());  

source

A string really is just a container of characters, and therefore you can apply the erase/remove idiom to it.

Answer 2

This should work. It's much easier to think about two pointers moving through the string than one pointer plus an offset.

auto sptr = lpsz;
auto dptr = lpsz;
while (*dptr = *sptr) { // copy up to and including terminating NUL
    if (*dptr != ' ') dptr++; // but spaces take no slots in the destination
    sptr++;
}

• Demo: http://ideone.com/Pmmc20

or even

auto sptr = lpsz;
auto dptr = lpsz;
while (auto c = *dptr = *sptr) {
    dptr += (c != ' ');
    sptr++;
}

The essential difference between this and the original code is that when we see something other than a space, both mine and the original move the read and write location forward one. But when we see a space, I move the read position forward by 1 and don't move the write position, while the original moves the read position forward by 2 and the write position forward by one, which skips a character. Also, the original is testing at the write location for a space, instead of at the read location (mine tests the write location, but it does so after copying the character from the read location, so the result is correct).

Answer 3

You should use algorithms for this, as they are well tested. But if you want to analyze your code and understand the failure consider a high level description of what your code is running (See Tony's answer).

You maintain two indices into the buffer, one for reading and one for writing. Whenever the reading head detects a space you move it but skip the write. If the character is not a space you use the read head to get a value and write it through the write head.

In your implementation the read head is ptr+i, which is spelled a bit strange as an offset from the write head (as in ptr[i]). The write head is ptr (*ptr =). But your test in the loop is using the write head instead of the read head: if (*ptr==' ').

Even if you fix this, the implementation has other issues, like for example the fact that if there are two consecutive spaces, since you do a single test in the loop. A rewrite of your algorithm could be:

char* remove_spaces(char* buffer) {
   char *read = buffer;
   for (char *read = buffer; (*read), ++read) {
      if (*read != ' ') {      // copy element
         *buffer = *read;
         ++buffer;             // and increment write head
      }
   }
   *buffer = 0;                // ensure null termination
   return read;
}

Now, the algorithm can be further improved (performance) by removing the number of writes to memory if you do an initial search for the first space and use that as the starting point for the loop above. That will reduce the number of operations and the number of cache lines that are marked dirty.

Answer 4

Step through your loop carefully.

i is set to 0 at the start of the loop. For the first space encountered, i is incremented (so i==1). The character at ptr is replaced with the character at prt+i, which is the next character. This works the first time, because i is 1.

But for the second space, i is set to 2 (since it's incremented), so the space is replaced by the character at ptr+2

It would be much easier to do this as part of a string copy, rather than an in-place alteration. dest is a destination buffer for our altered copy.

for(ptr = lpsz; *ptr; ptr++){
    if(' ' == *ptr) {continue;}
    *dest = *ptr;
    dest++;
}
*dest = 0;