0
#include <iostream>
#include <fstream>
#include <cstring>
#include <map>
using namespace std;

int main()
{
    cout << "Hello world!" << endl;

    //define a bool to determine if you're still in the file's header section or not
    bool header = true;
    //define the map as a multidimensional string array that stores up to 100 z-levels
    //and 50x50 tiles per z level
    char* worldmap[100][50][50];
    int zLevel=0;
    //header declaration map
    map<string, char*> declarations;
    //create an input file stream object
    ifstream file;
    //open file
    file.open("map1.dmm");
    //validate file
    if(!file.good()){return 1;}

    //begin reading the file
    while(!file.eof()){
        //create a character array to write to
        char line[512];
        //read the file line by line; write each line to the character array defined above
        file.getline(line, 512);

        //header check
        if(header){
            if(!line[0]){
                header = false;
                break;
            }else{
                bool declaringKey=true;
                char* key={};
                char* element={};
                char* token[20]={};
                token[0] = strtok(line, "\"()");
                for(unsigned int n = 0;n<20;n++){

                    if(n>0)token[n] = strtok(NULL, "\"()");
                    //cout << token[0] << endl;
                    if(!token[n] || (token[n])[1] == '=')continue;
                    if(declaringKey){

                        key = token[n];
                        declaringKey=false;

                    }else{
                        //cout << "pow" <<endl;
                        element = token[n];
                        cout<<"declarations[" << key << "] = " << element << endl;
                        declarations.emplace(key,element); //<-------------- problem line, i think
                        cout << declarations[key] << endl;

                    }

                }declaringKey=true;

            }
        }else{
            if(!line[0]) {
                zLevel++;
                continue;
            }

        }

    }
//string test = "aa";

    return 0;

}

I'm trying to create a map loader that loads a simple map from a text file. I'm aware that there are other map loaders available but most of them do far more than I need them to. So far, this code only reads the header, which basically defines what each set of characters represents as a tile, for example: "aa" = "sand tile"

The problem is, when I'm emplacing the key/element into the declarations map, it seems to use the same element for all keys. I'm assuming that this is because by defining a character pointer it always points to the same data, and only serves the purpose of changing the value contained by that pointer, rather than allocating new data and keeping them separate.

But that raises another question, why does it emplace a different key with the same element, even though both are pointers..? Anyways,

How can I make it so that all keys/elements are independent character arrays, rather than pointers to the exact same space carved out by the array..?

EDIT: You can just assume the code works other than the fact that it stores the same element to all keys. Also, the element it stores is always the last one that's parsed out of the file's header section.

gabriel.hayes
  • 2,267
  • 12
  • 15
  • 5
    If you're programming C++, the my advice is to stop using pointer as much as you can. – Some programmer dude Nov 29 '13 at 21:10
  • And to start with, there is one obvious problem with your code, and that is that you are doing `while (!file.eof())` which will not do what you expect it to do, as the `eofbit` is not set until *after* a read operation fail. Instead do e.g. `while (std::getline(...))`. And start using `std::string` for strings. – Some programmer dude Nov 29 '13 at 21:12
  • Just adding to what @JoachimPileborg said: You almost never need to use explicit pointers (using the syntax `*`,`&` etc) in C++. – keyser Nov 29 '13 at 21:14
  • @JoachimPileborg Actually the while loop works fine, it goes on until the end of file bit is set then stops, like I intended. – gabriel.hayes Nov 29 '13 at 21:15
  • @ᴋᴇʏsᴇʀ Wouldn't I have to..? strtok returns a pointer to a character array, thus it can only be stored in a pointer to a character array. Whenever I tried doing this without explicitly defining pointers, I got a bunch of conversion errors. – gabriel.hayes Nov 29 '13 at 21:17
  • If you have something that returns pointers you could simply dereference them. Or store them as pointers, of course. But as mentioned, when avoidable, avoid :) Don't take my word for it though, I'm no C++ guru. – keyser Nov 29 '13 at 21:19
  • 2
    @user1538301 See http://stackoverflow.com/questions/289347/using-strtok-with-a-stdstring for the equivalent of strtok in C++. Try to avoid mixing the C and the C++ API. Use std::string in C++, not char arrays. – Étienne Nov 29 '13 at 21:22
  • No really, the loop will loop once to many, and your `file.getline` call will fail without you checking for it. – Some programmer dude Nov 29 '13 at 21:28

1 Answers1

2

Just use a std::string for the value, too. This should solve your immediate problem.

That said, do not use stream.eof() to control a loop reading values! It does not work. Instead, always try to read from a stream and then verify if the read was successful, e.g.:

while (file.getline(line, sizeof(line))) {
    // ...
}

Personally, I wouldn't read into a fixed size buffer and use a std::string instead:

for (std::string line; std::getline(file, line); ) {
    // ...
}

From this point I would also not use strtok() but rather either the members of std::string or suitable algorithms. This way I also wouldn't let astray, considering it a good idea to store pointers (not to mention that I can't deal with pointers and, thus, my programs don't use them).

Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
  • That's strange because using eof() is how I learned IO at university. Is there any reason why this doesn't work? – lucas92 Nov 29 '13 at 21:51
  • 1
    @lucas92: sadly, I'm not surprised that you got taught this approach! The case which affects your code is that after the last line an empty line is processed. That may be harmless but is certainly not correct, e.g., when you try to exactly reproduce the content. There is a worse problem when any formatted value is read and it gets a wrong value (e.g., an `int` is read but a `a` is received): the stream goes into failure mode, won't make any further inputs, and won't ever set `std::ios_base::eofbit`. – Dietmar Kühl Nov 29 '13 at 21:58