Tokenize a string

I used strtok to tokenize a string. But many developers said me to avoid strtok, why? How should I optimize this code? Is it really wrong that I used strtok? If so then with what should I replace it?

DWORD FN_attack_speed_from_file(const char *file)
{
    FILE *fp = fopen(file, "r");
    if(NULL == fp)
    {
        return 0;
    }
    int speed = 1000;
    const char *key = "DirectInputTime";
    const char *delim = " trn";
    const char *field, *value;
    char buf[1024];
    while(fgets(buf, 1024, fp))
    {
        field = strtok(buf, delim);
        value = strtok(NULL, delim);
        if(field && value)
        {
            if(0 == strcasecmp(field, key))
            {
                float f_speed = strtof(value, NULL);
                speed = (int)(f_speed * 1000.0);
                break;
            }
        }
    }
    fclose(fp);
    return speed;
}

I would like to modernize this as C++, and learn modern coding.

I used strtok to tokenize a string. But many developers said me to avoid strtok, why?

Because strtok() modifies the input is the main reason.
But it has other issues that make it not a good option in modern code.

How should I optimize this code?

Well let us make it correct first before we go for optimizations. But strtok() will probably be the fastest. That’s because it basically does nothing but tokenization. But this also makes it dangerous to use.

Is it really wrong that I used strtok?

Yes if you are using C++. The better technique is to code for type safety and re-usability. The difference in speed will be minimal. So small that if this program is running for years that even the cumulative time saves by strtok() will not amount to the time taken to fix a single bug by a human.

If so then with what should I replace it?

You should probably use the C++ stream operators.

C++ Version

 // Slightly different to the original.
 // If the value part has an illegal character for a float it
 // will probably mess up.
 DWORD FN_attack_speed_from_file(char const* fileName)
 {
     std::ifstream file(fileName);

     std::string   field;
     float         value;
     while(file >> field >> value)
     {
         if (field == "DirectInputTime") {
             return value * 1000.0;
         }
     }
     return 1000;
}

Since you have indicated that you are using C++, you should use idiomatic C++ approaches to parse input. In particular, you should avoid strtok(), which is not recommended even in C, since it is not thread-safe.

It appears that you are looking to read whitespace-delimited words and floats. For that, use the >> operator on an ifstream.