-1

I know everybody is tired of being asked how to parse strings, but this is different. I am trying to make a command line interface for a drawing program in C++. I have been able to parse lines before but my problem is that there are multiple commands that have different amounts of parameters.

Here is my last attempt at my command line function (ignore the flag for now):

//Allow user to type in commands
void commandLine(){
    bool flag = true;
    do {
        cout << "Enter a command (format 'command x y z') : ";
        vector<string> v;
        string line;
        cin.ignore() //edit: seg fault without this
        getline(cin, line);
        char space = ' ';
        int p1, p2, p3, p4, p5, p6;
        auto i = 0;
        auto pos = line.find(space);
        while(pos != string::npos){
            v.push_back(line.substr(i, pos-i));
            i = ++pos;
            pos = line.find(space, pos);
            if (pos != string::npos)
                v.push_back(line.substr(i, line.length()));
        }
        if (v[0]=="size"){
            p1 = stoi(v[1]);
            p2 = stoi(v[2]);
            //call asm
            int ret = mysetSize(p1, p2);
            if (ret == 0){
                cout << "Something went wrong with the setSize command..." << endl;
            } else {
                cout << "Size set to " << p1 << "x" << p2 << endl;
            }
        }else if (v[0] == "clear"){
            //call asm
            int ret = myclear();
            if (ret == 0){
                cout << "Something went wrong with the clear command..." << endl;
            } else {
                cout << "Screen Cleared" << endl;
            }

        }else if (v[0] == "setBkgndColor"){
            p1 = stoi(v[1]);
            p2 = stoi(v[2]);
            p3 = stoi(v[3]);
            //call asm
            int ret = mysetBkgndColor(p1, p2, p3);
            if (ret==0){
                cout << "Something went wrong with the setBkgndColor command..." << endl;
            } else {
                cout << "Background color set to " << p1 << ", " << p2 << ", " << p3 << endl;
            }

        }else if (v[0] == "setDrawColor"){
            p1 = stoi(v[1]);
            p2 = stoi(v[2]);
            p3 = stoi(v[3]);
            //call asm
            int ret = mysetDrawColor(p1, p2, p3);
            if (ret == 0){
                cout << "Something went wrong with the setDrawColor command..." << endl;
            } else {
                cout << "Draw color set to " << p1 << ", " << p2 << ", " << p3 << endl;
            }

        }else if (v[0] == "rect"){
            p1 = stoi(v[1]);
            p2 = stoi(v[2]);
            p3 = stoi(v[3]);
            p4 = stoi(v[4]);
            vertex a;
            a.x = p1;
            a.y = p2;
            drawRect(a, p3, p4);

        }else if (v[0] == "circle"){
            p1 = stoi(v[1]);
            p2 = stoi(v[2]);
            p3 = stoi(v[3]);
            vertex a;
            a.x = p1;
            a.y = p2;
            drawCircle(a, p3);
        }else if (v[0] == "triangle"){
            p1 = stoi(v[1]);
            p2 = stoi(v[2]);
            p3 = stoi(v[3]);
            p4 = stoi(v[1]);
            p5 = stoi(v[2]);
            p6 = stoi(v[3]);
            vertex a, b, c;
            a.x = p1;
            a.y = p2;
            b.x = p3;
            b.y = p4;
            c.x = p5;
            c.y = p6;
            drawTriangle(a, b, c);
        }

    } while(flag == true);

}

When I type in a command, it just freezes with a blank line that I have to ctrl+c out of. Any ideas?

ANSWER: Evan Teran's method works for this from this link: Split a string in C++?

Community
  • 1
  • 1
Tyler Small
  • 85
  • 11
  • How about using an existing library for this? "Not invented here" is a common problem, you do not need to reinvent the wheel, when countless other people did before you already. There is e.g. [Boost Program Options](http://www.boost.org/doc/libs/release/libs/program_options/) – Jan Henke Nov 12 '16 at 20:46

1 Answers1

3

The problem is here:

    while(pos == string::npos){
        v.push_back(line.substr(i, pos-i));
        i = ++pos;
        pos = line.find(space, pos);
        if (pos != string::npos)
            v.push_back(line.substr(i, line.length()));

If you don't find a space, the returned value is npos, and you loop once more. But since there's no space at the end of your line, it continues indefinitely: infinite loop with memory increasing because as documentation states

If this (pos) is greater than the string length, the function never finds matches

So to sum it up:

  • if you enter a single command, without spaces, pos == string::npos, you enter the loop and you get an infinite loop as explained above
  • if you enter a command with arguments, you don't enter the loop since pos is not string::npos and your vector is empty => segfault.

I recommend you look at better ways to split your strings like here

But in the meanwhile I fixed your loop and it works fine now (and it's also simpler):

auto i = line.find_first_not_of(space);
for(;;){
    auto pos = line.find(space,i);
    // store token
    v.push_back(line.substr(i,pos-i));
    // no more space: bail out          
    if (pos==string::npos)
    {
        break;
    }
    // move to next non-space
    i = line.find_first_not_of(space,pos+1);
    // protection against trailing spaces
    if (i==string::npos)
    {
        break;
    }
}
  • while condition loops are often a bad idea because you have to perform the operation one outside the loop and the same operation within the loop, which is confusing
  • I just look for space character.
  • If not found, it returns string::npos so the substr extracts the whole string, then the loop stops.
  • If space found, it returns the index, skips to the first non-space and look for the next field.
  • If starts or/and ends with spaces, won't crash either :)
Community
  • 1
  • 1
Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219
  • Thank you for your answer. I tried that but now I got a segmentation fault as soon as it asks for a command. – Tyler Small Nov 12 '16 at 20:24
  • 1
    you have no guarantee about `v` size, but you're accessing `v[0]`, `v[1]` ... try to print `v.size()` and `v.contents()` – Jean-François Fabre Nov 12 '16 at 20:27
  • whoops I forgot to edit the while loop, but now it still does the same thing as before – Tyler Small Nov 12 '16 at 20:27
  • It either will get stuck in the while loop for eternity or it causes a segmentation fault within the while loop, so it won't even get to the point where I try to print v.size, v[0], and, v[1]. – Tyler Small Nov 12 '16 at 20:40
  • Thank you so much. You're gonna laugh. I needed a cin.ignore before the input which was causing a segmentation fault. I ended up using Evan Teran's method on the page you suggested, works great now! – Tyler Small Nov 12 '16 at 20:57
  • 1
    you're welcome. I have edited my answer to show the correct way of doing it "the hard way", with multiple/leading/trailing space handling, just for fun. – Jean-François Fabre Nov 12 '16 at 21:00