use of Range Based for loops in C++

Question

I am trying to write a program that eliminates blank spaces using a range based for loop in C++. For eg, if the input is, "what is your name?" , the output should be "Whatisyourname?" however when i run the code below, the output it gives is "Whatisyourname?me?", why is that?

int main()
{
    string s = "What is your name?";
    int write_index = 0;
    for (const char &c : s)
    {
        if (c != ' ')
        {
            s[write_index++] = c;
        }
    }
    cout << s << endl;
    system("pause");
}

Answer 1

This is a basic application of the copy_if algorithm from the standard library.

#include <algorithm>
#include <cctype>
#include <iostream>
#include <iterator>
#include <string>

int main()
{
    std::string s = "What is your name?";
    std::copy_if(s.begin(), s.end(), std::ostream_iterator<char>(std::cout),
        [](char c){ return !std::isspace(c); });
    return 0;
}

outputs:

Whatisyourname?

If you actually need to remove them from the original string, then use the algorithm remove_if followed by erase.

Answer 2

Now, I don't personally code C++, but this looks eerily similar to a for-each loop in C#, Java, and JavaScript; so I'll give it a go.

Let's first break down your code to see what's going on

int main() {
  // creates a string object which is essentially a glorified array of chars
  string s = "What is your name?"; 
  int write_index = 0;
  // for evry char "c" in the char-array "s"
  for (const char &c : s) {
    // if c isn't a space
    if (c != ' ') {
      // write c to s at index "write_index" then increment "write_index"
      s[write_index++] = c;
    }
  }

  std::cout << s << std::endl;
  system("pause");
}

The logic seems good, so why does "what is your name?" turn into "whatisyourname?me?"? Simple. Because you're overwriting the existing array.

"what is your name?" is 18 characters long, and since you're only writing a non-space character to the array if it's not a space you're essentially copying characters one space left for every space in your text.

For example here's what happens after you run this code over the first 7 characters: "whatiss your name?", and after the first 12: "whatisyourur name?", and finally after all 18: "whatisyourname?me?". The length of the string never really changes.

So you got a number of options to solve this issue:

1. Build a new string from the old one with a string-builder (if such a thing exists in C++) and return the freshly created string.
2. Count the number of spaces you encounter and return a substring that is that many characters shorter (original is 18 chars - 3 spaces = new is 15 chars).
3. Reduce the length of the string by the required amount of characters (Thanks Yakk for this one)

Answer 3

Here are two useful little functions:

template<class C, class F>
bool remove_erase_if( C& c, F&& f ) {
  using std::begin; using std::end;
  auto it = std::remove_if( begin(c), end(c), std::forward<F>(f) );
  if ( it == c.end())
    return false;
  c.erase( it, c.end() );
  return true;
}
template<class C, class T>
bool remove_erase( C& c, T&& t ) {
  using std::begin; using std::end;
  auto it = std::remove( begin(c), end(c), std::forward<T>(t) );
  if ( it == c.end())
    return false;
  c.erase( it, c.end() );
  return true;
}

these both take a container, and either a test or an element.

They then remove and erase any elements that pass the test, or equal the element.

Your code emulated the remove part of the above code, and did not do the erase part. So the characters remaining at the end ... remained.

remove (or your code) simply moves all the "kept" data to the front of the container. The stuff left over at the end ... stays there. The erase step then tells the container that the stuff after the stuff you kept should be discarded. If you don't discard it, it ... remains ... and you get your bug.

With the above two functions, you can do this:

 int main() {
  std::string s = "What is your name?";
  remove_erase( s, ' ' );
  std::cout << s << '\n';
}

and you are done.

As an aside, using namespace std; is often a bad idea. And std::endl forces a buffer-flush, so I prefer '\n'. Finally, system("pause") can be emulated by running your IDE in a mode that leaves you your command window open, instead of adding it to your code Ctrl-F5.

Answer 4

Try this:

#include <string.h>
#include <iostream>

using namespace std;

int main()
   {
       string s = "What is your name?";
       std::string aux(s.size(),' ');
       int write_index = 0;
       for (const char &c : s)
       {
               if (c != ' ')
               {
                   aux[write_index++] = c;
               }
       }
       cout << s << endl;
       cout << aux << endl;
       system("pause");
   }

Answer 5

Add after the loop the following statement

s.erase( write_index );

or

s.resize( write_index );

to remove redundant characters from the string.

The general approach to such tasks is the following

#include <algorithm>
#include <string>

//...

s.erase( std::remove( s.begin(), s.end(), ' ' ), s.end() );

Answer 6

The reason for this is because string s is still as long as the original string, "What is your name?". You wrote over top of every character in the string except for the last three. What you could do is erase the last three characters from the string after you're done removing the spaces. This is untested but something like this should work:

s.erase(write_index, s.length() - write_index)

Your range based for loop usage is correct. Just keep in mind that you're looping over all the input characters (as though you were looping with for (int i = 0; i < s.length(); i++), but you're not outputting as many characters as you're reading.

So the equivalent for loop would be like this:

for (int i = 0; i < s.length(); i++) {
    const char& c = s[i];

    if (c != ' ') {
        s[write_index++] = c;
    }
}

Answer 7

You can keep track of the number of spaces you have and resize the string at the end.

int main()
    {
        string s = "What is your name?";
        int length = s.length();
        int write_index = 0;
        for (const char &c : s)
        {
                if (c != ' ')
                {
                    s[write_index++] = c;
                }
                else
                {
                    length -= 1;
                }
        }
        s.resize(length);
        cout << s << endl;
    }