C++ splitting string using strtok ignores the last entry

Question

I have the following code that is suppose to split a string and inserting it into an array :

 char *t; 
 char *tmpLine = (char *)line.c_str();
 t = strtok(tmpLine, "\t"); 
 int counter = 0;
 while(t != NULL) { 
     tempGrade[counter] = atoi(t);
     counter++;
     t = strtok(NULL, "\t"); 
 }

but for some reason the last entry of line is ignored and not inserted. also line is :

string line = "1 90 74 84 48 76 76 80 85"; NOTE : the spaces are tabspaces (\t) in the original file.

Answer 1

Your code is so wrong it burns my eyes.

What is so wrong?

char *tmpLine = (char *)line.c_str();

First, you are (unless I'm wrong) casting away the constness of a std::string, which is a very bad idea. Anyway, casting away the constness smells of modification attempt...

t = strtok(tmpLine, "\t");

And here we are...

You are modifying the buffer provided by the std::string, as strtok destroys the string given to it.

Bad idea: You are not the owner of the std::string's internal buffer, so you should not modify it (you don't want to break your string, and provoke a bug, want you?),

Back to the code:

t = strtok(tmpLine, "\t");

Ok, so, you're using strtok, which is not a reentrant function.

I don't know what compiler you're using, but I guess most would have a more safe (i.e. less stupid) alternative. For example, Visual C++ provides strtok_s. If you don't have one provided, then the best solution is to write your own strtok, or use another tokenization API.

In fact, strtok is one of the few counter-examples of "don't reinvent the wheel": In that case, rewriting your own will always be better than the original standard C function, if you have some experience in C or C++.

Ok, but, about the original problem?

Others have provided insight about alternatives, or even the fact the incomplete sample of code you provided seemed complete, so I'll stop here my analysis, and let you consider their answers.

I don't know what your code is for, but it is plain wrong, and would be even if the bug you reported (the missing token) didn't exist. Don't use that on production code.

Answer 2

but can you give me a quick hack around this? because this is not an important project and I would have to change alot to do it like in that question

No, you don't. This does the same thing assuming tempGrade is an int array:

istringstream iss(line);
int counter = copy(istream_iterator<int>(iss),
         istream_iterator<int>(),
         tempGrade) - tempGrade;

But it would be better to change tempGrade to vector and use the code from the answer Moo-Juice linked to.

Answer 3

As other people mention, I suggest you take the recommended C++ approach to this problem.

But I tried out your code, and I can't tell what is wrong with it. I don't repro your issue on my end:

1
90
74
84
48
76
76
80
85

Perhaps your iteration to print out the loop is too short, or your tempGrade array is too small? Or perhaps your final "tab" isn't really a tab character?

Here's the code I compiled to check this code.

#include<iostream>

int main(int argc, char* argv[])
{
    std::string line = "1\t90\t74\t84\t48\t76\t76\t80\t85";
    char *t; 
    char *tmpLine = (char *)line.c_str();
    t = strtok(tmpLine, "\t"); 
    int counter = 0;
    int tempGrade[9];
    while(t != NULL) { 
        tempGrade[counter] = atoi(t);
        counter++;
        t = strtok(NULL, "\t"); 
    }
    for(int i = 0; i < 9; ++i) {
        std::cout << tempGrade[i] << "\n";
    }
}

Answer 4

Here is a very simple example showing how to make use of stream extraction operators and vector to do this in a more idiomatic C++ manner:

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

using namespace std;

void parse(string line)
{
    stringstream stream(line);
    int score;
    vector<int> scores;
    while (stream >> score)
        scores.push_back(score);

    for (vector<int>::iterator itr = scores.begin(); itr != scores.end(); ++itr)
        cout << *itr << endl;
}

int main(int argc, char* argv[])
{
    parse("1\t90\t74\t84\t48\t76\t76\t80\t85");
    return 0;
}