3

Possible Duplicate:
Splitting a string in C++

Im trying to create a function that mimics the behavior of the getline() function, with the option to use a delimiter to split the string into tokens. The function accepts 2 strings (the second is being passed by reference) and a char type for the delimiter. It loops through each character of the first string, copying it to the second string and stops looping when it reaches the delimiter. It returns true if the first string have more characters after the delimiter and false otherwise. The position of the last character is being saved in a static variable. for some reason the the program is going into an infinite loop and is not executing anything:

const int LINE_SIZE = 160;
bool strSplit(string sFirst, string & sLast, char cDelim) {
    static int iCount = 0;
    for(int i = iCount; i < LINE_SIZE; i++) {
        if(sFirst[i] != cDelim)
            sLast[i-iCount] = sFirst[i];
        else {
            iCount = i+1;
            return true;
        }
    }
    return false;
}

The function is used in the following way:

while(strSplit(sLine, sToken, '|')) {
    cout << sToken << endl;
}

Why is it going into an infinite loop, and why is it not working? I should add that i'm interested in a solution without using istringstream, if that's possible.

Community
  • 1
  • 1
Yoav Kadosh
  • 4,807
  • 4
  • 39
  • 56
  • 1
    As I read your post, I see no questions, only statements. Do you have a question for us? – Robᵩ Sep 12 '12 at 16:57
  • 3
    @Rob probably the infinite loop was not desired outcome. – Öö Tiib Sep 12 '12 at 16:58
  • Not directly to your current problem. Do you want this function to work only a single sLine? The static variable is not reset for a second sLine. – PermanentGuest Sep 12 '12 at 17:00
  • @ÖöTiib - I agree. But that doesn't excuse the poster from asking a question. In my experience, posts without questions result in a confusing array of answers, each answering a different (unstated) question. – Robᵩ Sep 12 '12 at 17:01
  • @Robᵩ I accepted your comment and edited my original post. – Yoav Kadosh Sep 12 '12 at 17:07
  • see this for a multitude of solutions to your problem: http://stackoverflow.com/q/236129/1025391 – moooeeeep Sep 12 '12 at 17:08
  • @PermanentGuest - This function should work for multiple lines, and should reset the static variable for each line. – Yoav Kadosh Sep 12 '12 at 17:08
  • 1
    Again not the question you asked (that's been answered already) but the design of this function is terrible. It's the static variable that is the problem, it means that the function cannot be used in a nested fashion. That is you cannot have an outer loop and an inner loop both calling strSplit because both loops will be sharing the same static variable. Here's a better way to design this function. Have the function remove the split string and the delimiter from sFirst. That way strSplit can always start at the beginning of sFirst and you don't need the static variable. – john Sep 12 '12 at 17:16
  • 1
    Congrats for trying to write your own utility functions though. Clearly you have what it takes. – john Sep 12 '12 at 17:17
  • @john both, the OP's and your approach won't be thread-safe in the end. – moooeeeep Sep 12 '12 at 17:27
  • @moooeeeep No-one's asking for thread safety. – john Sep 12 '12 at 17:31

4 Answers4

5

It is not exactly what you asked for, but have you considered std::istringstream and std::getline?

// UNTESTED
std::istringstream iss(sLine);
while(std::getline(iss, sToken, '|')) {
  std::cout << sToken << "\n";
}

EDIT:

Why is it going into an infinite loop, and why is it not working?

We can't know, you didn't provide enough information. Try to create an SSCCE and post that.

I can tell you that the following line is very suspicious:

       sLast[i-iCount] = sFirst[i];

This line will result in undefined behavior (including, perhaps, what you have seen) in any of the following conditions:

  • i >= sFirst.size()
  • i-iCount >= sLast.size()
  • i-iCount < 0

It appears to me likely that all of those conditions are true. If the passed-in string is, for example, shorter than 160 lines, or if iCount ever grows to be bigger than the offset of the first delimiter, then you'll get undefined behavior.

Robᵩ
  • 163,533
  • 20
  • 239
  • 308
3

LINE_SIZE is probably larger than the number of characters in the string object, so the code runs off the end of the string's storage, and pretty much anything can happen.

Instead of rolling your own, string::find does what you need.

std::string::size_type pos = 0;
std::string::size_type new_pos = sFirst.find('|', pos);

The call to find finds the first occurrence of '|' that's at or after the position 'pos'. If it succeeds, it returns the index of the '|' that it found. If it fails, it returns std::string::npos. Use it in a loop, and after each match, copy the text from [pos, new_pos) into the target string, and update pos to new_pos + 1.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
1

are you sure it's the strSplit() function that doesn't return or is it your caller while loop that's infinite?

Shouldn't your caller loop be something like:

while(strSplit(sLine, sToken, '|')) {
    cout << sToken << endl;
    cin >> sLine >> endl;
}

-- edit --

if value of sLine is such that it makes strSplit() to return true then the while loop becomes infinite.. so do something to change the value of sLine for each iteration of the loop.. e.g. put in a cin..

Kashyap
  • 15,354
  • 13
  • 64
  • 103
0

Check this out

std::vector<std::string> spliString(const std::string &str, 
                                    const std::string &separator)
{
    vector<string>      ret;
    string::size_type   strLen = str.length();
    char                *buff;
    char                *pch;

    buff = new char[strLen + 1];
    buff[strLen] = '\0';
    std::copy(str.begin(), str.end(), buff);

    pch = strtok(buff, separator.c_str());
    while(pch != NULL)
    {
        ret.push_back(string(pch));
        pch = strtok(NULL, separator.c_str());
    }

    delete[] buff;

    return ret;
}
micnyk
  • 726
  • 8
  • 27