Memory allocation and deallocation

[parallx]

setMap(): This method receives the name of a text file that contains an ASCII map. The map will be a square, equal number of rows and columns. The first line of the map will have the number of rows. Every line after will contain a number of ASCII characters that you must read into the map. This must allocate memory before assigning the map.

Any idea what to do ?

here is my code:

void vehicle::setMap(string s) {

    ifstream infile;
    int matrix_size;
    string temporary;

    infile.open(s);

    if (!infile)
    {
        cout<<"File open failure."<<endl;
    }
    else
    {
        infile>>matrix_size;
        map = new char *[matrix_size];
        for (int i = 0; i < matrix_size; ++i) {
            map[i] = new char();
        }

        for (int j = 0; j < matrix_size ; ++j) {
            infile>>temporary;
            for (int i = 0; i < matrix_size; ++i) {
                map[i][j] = temporary.at(i);
            }
        }
    }
    infile.close();

}

Just gives me malloc errors.

operator--: The overload of this operator will deallocate the memory allocated for the map.

Not sure how to do this based on the allocation not working.

[doug4]

Line 18, you are only allocating a single character. So you now have maxtrix_size rows of char* each pointing to a single char.

Change line 18 to map[i] = new char[matrix_size];

[poteto]

I assume this map is for a 2d map for a game.

Do you really think it is smart to allow people to make certain rows to have a different sizes than others? Because that's what you are allowing.

This would be the biggest pain in the ass to error check, especially because this gives a segfault if something is wrong.

And note that if you loop through your map frequently, the jagged array will cause many cache misses.

And I notice you are not keeping the size of the matrix outside the function, which I am curious, how do you plan on getting the size????

And please for the love of god check if map == NULL and delete it if it isn't, you should have a macro called SAFE_DELETE, defined like SAFE_DELETE(x) if(x) delete x; x=NULL;. Or if you are very confident it wont be called twice, assert(!map && "bad");.

What I suggest is to use a true 2d array (which is how a 2d array looks like with compile time dimensions IE: int arr[3][4];), and use a 1d treated as a 2d one (you may doubt me, but it really is the only good way). You can wrap this array in a class which contains the width, height and overload the operator() to index the array using x+y*width, and use a vector so that you can catch exceptions instead of seg faulting. To load the array, you will have both the width and height on top of the file so you can error check, and just load it up on a 1d array, which is much simpler than an array of arrays, and keeps your data tight and it tells other people easily what type of data it is supposed to contain (as in, a 2d array that looks like a box, not jagged).

You can even make the "2d array object" into a "map" object and have it include the function to load the map.

And I should also note that using streams may work, but from my perspective it would seem much more neater to have the whole file in 1 string am I right?

Then you can just copy the file into a string or VLA(not VS compatible) using the ifstreams write function (don't forget to check if the ifstream is good() after loading too. (not good for gigabyte big files, but I doubt that is your intention).

Even if it means ignoring every newline and having to handle it on a lower level, it isn't as bad as you think, and you can also error check the width and height and compare it to the size of the file, but note I would give an error if it was too small, and a warning if it is too big, because maybe someone puts in an extra newline or space at the end.

But I should warn you that what I said above could be NIL'd easily if you put a space in between each tile, solves a couple problems and keeps things simple (just make sure your stream is still valid by checking if(file>>i)).

[parallx]

potato to answer all your questions in a simple manner, I haven't a clue as what I am doing and I don't understand your questions. I'm rather useless at C++ so I'm literally figuring this out from google and a sub par textbook which isn't helpful. Lectures always answer my questions with other questions which isn't helpful. I can't use vectors because I have not been taught them. I would have given the whole question but last time I did that a user "very kindly" said I mustn't do that. Thanks for the suggestions, I'll carry on and see what I can figure out based on the info you've provided.

doug, I'll give that a gander and see what I come up with.

[kbw]

setMap(): This method receives the name of a text file that contains an ASCII map.

This gives you a signature:

void setMap(std::string filename) {
}

The map will be a square, equal number of rows and columns. The first line of the map will have the number of rows.

You need to open the file and read the first line and extract the number of rows.

void setMap(std::string filename) {
  std::ifstream is(filename);
  if (is) {
    std::string line;
    std::getline(is, line);
    int nrows = atoi( line.c_str() );
  }
}

Every line after will contain a number of ASCII characters that you must read into the map. This must allocate memory before assigning the map.

There are a number of ways you can represent a map.

As it's of a known fixed length, you can allocate a single block of memory and store it there. The size of the block would be nows x nrows. But you need to calculate the index into the map. The index of position [row, col] might be row*nrows + col.

Alternatively, you can implement it as an array of arrays. You need to allocate the first array, then allocate the other arrays individually. For example:

typedef char map_entry;  // our map has chars
typedef char* map_column;  // this is a column
typedef map_column* map_row;  // this is a row.
typedef map_row* map;  // this is our map

// create a map.
map* create_map(int nrows, int ncols) {
  // create row
  map* local_map = new map_row[ nrows ];

  // create columns
  for (int col = 0; col < ncols; ++col)
    local_map[ col ] = new map_column[ ncols ];

  return local_map;
}

Let's use the first method. We have to calculate the index ourselves, but the allocation is a lot easier.

void setMap(std::string filename) {
  std::ifstream is(filename);
  if (is) {
    // read the size from the first line
    std::string line;
    std::getline(is, line);
    int nrows = atoi( line.c_str() );

    // read "nrows" lines into our map.
    char* map = new char[ nrows * nrows ];
    for (int row = 0; row < nrows; ++row) {
      // the characters in a line can be read into a string, rather than reading them one at a time.
      std::getline(is, line);
      for (int col = 0; col < nrows && col < line.size(); ++col)
        map[ row*nrows + col ] = line[col];
    }

    // we're done
    return;
  }

  // couldn't open file
  std::cerr << "cannot open file: " << filename << std::endl;
}

I hope that helps.

[parallx]

kbw that makes more sense.

Would the deletion of memory also be a for loop to remove the columns and the rows and then delete the whole map after that ?

[kbw]

Yes.

[Cubbi]

parallx wrote:

I'm literally figuring this out from google and a sub par textbook [...] I can't use vectors because I have not been taught them

Why not replace that "sub par textbook" with one written by someone who has a clue, such as Stroustrup's PPP, or the C++ Primer (not "Primer Plus")? You'd at least learn about beginner content like vectors in the beginning.
(kudos to kbw for putting together a good answer despite the unrealistic requirements)

[parallx]

Cool thanks kbw.

Cubbi I'll look them up, thanks.