C++ After while loop size of map is still 1

Question

My question is that after the while loop, the size of my "mapFile" is still 1. I don't know where I am getting it wrong.

This is my code:

using namespace std;

int main(void)
{
    char buffer[1024];
    char *record, *line;

    map<char *, vector<char *>> mapFile;
    vector<char *> myVector;

    char* fileName = "TestFile.csv";

    FILE *fstream;

    if(( fstream = fopen(fileName, "r")) == NULL) {
        printf("File could not be opened\n");
        return -1;
    }
    else{
        while((line = fgets(buffer, sizeof(buffer), fstream)) != NULL)
        {
            record = strtok(line, ",");
            char* temp = record;

            myVector.push_back(record);
            while(record != NULL)
            {
                record = strtok(NULL, ",");
                myVector.push_back(record);
            }

            mapFile.insert(pair<char *, vector<char *>>(temp, myVector));           
        }
        fclose(fstream);
    }

    return 0;
}

`temp` is an address inside of `buffer`. You re-use buffer each time you loop. No copies. — ta.speot.is, Jan 27 '14 at 10:05
Debugging tip: how many *unique* values of `temp` are you getting? Print `(void*) temp` and look. — molbdnilo, Jan 27 '14 at 10:06

score 2 · Accepted Answer · edited May 23 '17 at 10:25

You are using the address of record which is held by the variable temp as the key in your map. Its address is always the same so subsequent additions are ignored because multiple keys are not allowed in a map.

A simple solution would be to replace all the pointer to null terminated strings (char* typed variables) with std::strings.

Using std::strings rather than C strings is considered a more modern C++ coding style and is far less error prone. The code below is semantically similar to yours but compiles and runs under MSVC2013. I would also recommend using regex rather than strtok or similar C string parsing functions. Regex is a bit of intellectual overhead up front but it pays off in the long run as it is less error prone and more powerful.

Another side note, using nullptr rather than NULL is considered better if your compiler supports it. see What exactly is nullptr?

using namespace std is also considered bad practice, see : Why is "using namespace std" considered bad practice?

Here is the code:

#include <iostream>
#include <string>
#include <map>
#include <vector>
#include <cstring>
int main()
{
        char buffer[1024];
        char * line;

        std::map<std::string, std::vector<std::string>> mapFile;
        std::vector<std::string> myVector;

        const std::string fileName{ "E:\\test project\\ConsoleApplication1\\Debug\\TestFile.csv" };

        FILE *fstream;

        if ((fstream = fopen(fileName.c_str(), "r")) == NULL) {         //should at least use fopen_s here, see https://stackoverflow.com/questions/906599/why-cant-i-use-fopen
            printf("File could not be opened\n");
            return -1;
        }
        else{
            while ((line = fgets(buffer, sizeof(buffer), fstream)) != NULL)
            {
                char * result = strtok(line, ",");  //stdtok is very unsafe and outdated, should use regex something simmilar 
                while (result != nullptr){
                    myVector.push_back(std::string(result));
                    result = strtok(nullptr, ",");  //stdtok is very unsafe and outdated, should use regex something simmilar 
                }
                if (myVector.empty()){
                    //TODO handle error here, may be a line like ",,,"
                }
                else{
                    mapFile.insert(std::pair<std::string, std::vector<std::string>>(myVector[0], myVector));
                }
            }
            fclose(fstream);
        }

        return 0;
}

But when I debug, the value of temp is changed following by record. How can I fix it? — mrdlf, Jan 27 '14 at 10:11
There is another bug is the original and this code: strtok may return NULL (Eg if the record is ",,,,") — , Jan 27 '14 at 10:30
@DieterLücking yes, that is another reason he should be using regex ;) — odinthenerd, Jan 27 '14 at 10:35
Im on my tablet in a train right now, I can debug in a hour or so if you havn't found the problem by then. — odinthenerd, Jan 27 '14 at 10:37
Anyway, many thanks for hopeful knowledge! I'll try to change follow my own way based on your comments. — mrdlf, Jan 27 '14 at 10:41

score 0 · Answer 2 · answered Jan 27 '14 at 10:16

0

The beast is strtok: It is not reentrant and returns a pointer to an internal buffer (which is not changing) in this case. Make your mapFile a map< std::string, std::vector< std::string > >

answered Jan 27 '14 at 10:16

But after I changed like that. function insert() of map class mentioned error.Error 4 error C2784: 'bool std::operator <(const std::_Tree<_Traits> &,const std::_Tree<_Traits> &)' : could not deduce template argument for 'const std::_Tree<_Traits> &' from 'const std::string'. It's so difficult to understand. – mrdlf Jan 27 '14 at 10:25

C++ After while loop size of map is still 1

2 Answers2