Tyler Small Tyler Small - 21 days ago 9
C++ Question

Parsing a line with C++

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++?

Answer

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 :)
Comments