I'm having difficulties splitting a string into a vector

Question

I'm trying to write code that initializes a string into a vector by splitting it at every space character. Somehow the temporary vector won't take the positions and it doesn't split the string right.

#include <iostream>
#include <string>
#include <vector>

std::vector<std::string> splitStr(std::string s, char cut = 32) {
    std::string temp = s;
    int begin = 0;
    std::vector<std::string> v;
    for (int i = 0; i < temp.length() - 1; ++i) {
        if ((int) cut == (int) temp[i]) {
            v.push_back(temp.substr(begin, i));
            begin = i + 1;
        }
    }
    return v;
}


using namespace std;


int main() {

    for (string e : splitStr("Hello this is a test!", ' ')) {
        cout << e << endl;
    }

}

I think it goes wrong while appending to the vector, but I don't get why that is. Can any of you guys tell me what I'm doing wrong?

See [this](https://stackoverflow.com/questions/236129/how-do-i-iterate-over-the-words-of-a-string) for how to split a string by spaces — NathanOliver, Sep 25 '19 at 19:52
Thank you that helps, but I still would like my code to work. — Demonic-Coder, Sep 25 '19 at 19:56
The second parameter on `std::string::substr` is for length. That means you need to change `v.push_back(temp.substr(begin, i));` to `v.push_back(temp.substr(begin, i - begin));` and then of course you need to take care of the last word. — DimChtz, Sep 25 '19 at 19:59
I see 2 problems right away : your code will push empty strings to the vector in case there are adjacent spaces + the last word is never added to the vector — Arne J, Sep 25 '19 at 20:02
Why `i < temp.length() - 1;`? and not `i < temp.length() ;`? or at least `i <= temp.length() - 1;` — DimChtz, Sep 25 '19 at 20:02
@Demonic *but I still would like my code to work.* -- What if there are two or more adjacent spaces? What is the output supposed to look like? — PaulMcKenzie, Sep 25 '19 at 20:16
@Demonic If any of the answers helped you to solve the problem, please accept it so this question is removed from the list of unanswered questions. — Ted Lyngmo, Sep 26 '19 at 16:28

PapaDiHatti · Answer 1 · 2019-09-25T20:25:33.123

The second argument to substr is number of characters you want to pick from original string and if that is not passed it will take all characters till end of string. Here is your code with those corrections :

  std::vector<std::string> splitStr(std::string s, char cut = 32) {
    std::string temp = s;
    int begin = 0;
    // Remove multiple spaces based on logic described here : https://stackoverflow.com/questions/8362094/replace-multiple-spaces-with-one-space-in-a-string
    std::string::iterator new_end = std::unique(temp.begin(), temp.end(), [](char one,char two) { return (one == two) && (one == ' '); } );
    temp.erase(new_end, temp.end());
    std::vector<std::string> v;
    for (int i = 0; i < temp.length() - 1; ++i) {
        if ((int) cut == (int) temp[i]) {
            v.push_back(temp.substr(begin, i-begin));
            begin = i + 1;
        }
    }
    v.push_back(temp.substr(begin));
    return v;
}
    using namespace std;


    int main() {

        for (string e : splitStr("Hello this is a test!", ' ')) {
            cout << e << endl;
        }

    }

This won't give the desired results if there are two or more adjacent spaces. — PaulMcKenzie, Sep 25 '19 at 20:11
I asked the OP in the original comment section what they want to show as output if two or more spaces are encountered. — PaulMcKenzie, Sep 25 '19 at 20:17
@PaulMcKenzie thanks for the comment, implemented the change that you suggested — PapaDiHatti, Sep 25 '19 at 20:26

score 0 · Answer 2 · answered Sep 25 '19 at 20:26

For starters do not use magic numbers like 32.

Secondly the tab character also can be considered as a blank separator. The first parameter should be a const reference to a string.

There is no need to create a copy of the parameter within the function.

This loop

for (int i = 0; i < temp.length() - 1; ++i) {
    if ((int) cut == (int) temp[i]) {
        v.push_back(temp.substr(begin, i));
        begin = i + 1;
    }
}

does not make sense. For example a string can contain two consecutive blanks. Secondly the second argument of the call of substr temp.substr(begin, i) is invalid. It shall specify the length of the substring,

Instead of the loop use standard searching member functions of the class std::string.

Here is a demonstrative program that shows how the function can be implemented.

#include <iostream>
#include <string>
#include <vector>

std::vector<std::string> splitStr( const std::string &s, 
                                   const std::string &delim = " \t" )
{
    std::vector<std::string> v;

    for ( std::string::size_type pos = 0, n = 0;
          ( pos = s.find_first_not_of( delim, pos ) ) != std::string::npos; 
          pos += n )
    {
        n = s.find_first_of( delim, pos );

        n = ( n == std::string::npos ? s.size() : n ) - pos;

        v.push_back( s.substr( pos, n ) );
    }

    return v;
}

int main() 
{
    for ( const auto &s : splitStr( "Hello this is a test!" ) )
    {
        std::cout << s << std::endl;
    }

    return 0;
}

Its output is

Hello
this
is
a
test!

score 0 · Answer 3 · answered Sep 25 '19 at 20:27

splitting it at every space character

This will split it at every space character. I've commented on the changes in code.

#include <iostream>
#include <string>
#include <vector>

// don't use magic numbers like 32 for space. Use a literal space: ' '

// Instead of copying the string (potentially twice), use a reference to
// your input parameter and make it const if you are not going to change it.

std::vector<std::string> splitStr(const std::string& temp, char cut = ' ') {
    // Use types designed for the task to avoid casting, like
    // size_t for most indexing tasks.
    size_t begin = 0;
    std::vector<std::string> v;

    // with temp.length()-1 you won't capture a space at the very end
    for(size_t i = 0; i < temp.length(); ++i) {
        if(cut == temp[i]) {
            // the second argument to substr is the length of the sub string
            v.push_back(temp.substr(begin, i - begin));
            begin = i + 1;
        }
    }
    // add the remaining part of the string
    v.push_back(temp.substr(begin));
    return v;
}

// using namespace std; // don't do this globally

int main() {
    // added double spaces between words and one at the end for effect
    for(const std::string& e : splitStr("Hello  this  is  a  test! ", ' ')) {
        // added > and < around the words to see what you captured
        std::cout << '>' << e << '<' << std::endl;
    }
}

Output:

>Hello<
><
>this<
><
>is<
><
>a<
><
>test!<
><

This is making me a better programmer, thanks for the effort I appreciate it. — Demonic-Coder, Sep 25 '19 at 20:42

I'm having difficulties splitting a string into a vector

3 Answers3