1

my problem is that I have defined a string like so:

string messages = "This is an option; This is a really long option; Another One For Testing Sake; This is the last one I swear; You lied to me!";

The ';' characters in the string are to be treated like delimiters. In the grand scheme of things this string is called into a function res.addMessages(messages); The code for which is:

void ConflictResMenu::addMessages(string messages) {

    int idx = 0;
    for (int i = 0; i < messages.length(); i++) {

        cout << messages.find_first_of(';') << endl;
        if (messages[i] == this->delim) {

            this->split_messages.push_back(messages.substr(idx, i));
            idx = i + 1;

        }
    }
}

The problem with this is that the if clause is called at all the wrong times so the output ends up like:

This is an option
This is a really long option; Another One For
Another One For Testing Sake; This is the last one I swear; You lied to me!
This is the last one I swear; You lied to me!

I honestly have no idea what is going on here. If anyone can help or suggest a better way of approaching this, I would be very grateful.

PurityLake
  • 1,110
  • 10
  • 18
  • `for (int i = 0; i < messages.length(); i++) {` you cannot do this, it's wrong, check if next delimiter exists instead – UnknownError1337 Aug 27 '13 at 11:32
  • Without knowing, for example, what `this->delim` is, how can we know what the result is? Your code looks about right to me, so there's probably something else wrong here. Of course, it does stop short of the last string if there is no `;` on the end. But I don't see why it would stop in the middle - something else is going on. Please produce a small complete program. – Mats Petersson Aug 27 '13 at 11:36
  • I did specify that the delimiter is ';' – PurityLake Aug 27 '13 at 11:37

3 Answers3

4

You might use std::istringstream and std::getline to split the string:

std::istringstream is(messages);

std::string msg;
while (std::getline(is, msg, ';'))
{
    split_messages.push_back(msg);
}
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 1
    @PurityLake My solution doesn't remove the leading spaces after the semicolon, but see e.g. [this old SO answer](http://stackoverflow.com/a/217605/440558) for functions to trim whitespace from strings. – Some programmer dude Aug 27 '13 at 11:44
  • Nice one! :) There are so many usefull functions added in C++, you probably need a lifetime for knowing them all. :) – Devolus Aug 27 '13 at 11:45
3

The actual problem in your code is that you are not calculating the length properly.

Here's a function that I tried out:

void ConflictResMenu::addMessages(std::string messages) {

    int idx = 0;
    for (int i = 0; i < messages.length(); i++) {
        if (messages[i] == this->delim) 
        {
            this->split_messages.push_back(messages.substr(idx, i-idx));
            idx = i + 2;
        }
    }
    if (idx <= messages.length())
    {
        this->split_messages.push_back(messages.substr(idx));
    }
}

(I also used idx = i+2 to remove the space after the ;)

As others have pointed out, using find_first_of() would work at least as well, using a starting position.

Mats Petersson
  • 126,704
  • 14
  • 140
  • 227
2

find_first_of has an overload that takes a position. You don't need the loop at all - or the if statement.

Your problem is that you're throwing away the result of find_first_of, which you need to use to determine the sub-string end position, and you need subsequent searches to start at that position. When you're done, find_first_of returns npos. A while loop based on that condition, along with the position iterators, should give you what you want.

SteveLove
  • 3,137
  • 15
  • 17
  • `find_first_of` is not needed in this example, because he uses it just for printing it. the actual code is in the loop, which looks IMO right. You are right, that a `while` loop may be more readble, but that's not the problem here. – Devolus Aug 27 '13 at 11:42