3

I just need to be sure I have enough memory available to the bi-dimensional array map. So I think I'd have to put a try catch bad:alloc in every row of map or perhaps with no throw would be more elegant. But I can't get with it. Any help would be appreciated...

I've read some articles about writing something like:

 map[i] = new int [columns];
 if (map[i] == NULL)
 cout << “No enough memory”;

I don't like this solution and I've read that is not very reliable one. fillmaze is a function called inside the constructor of maze..

void Maze::setupMaze(int r, int c, int l){
    rows = r;
    columns = c;
    level = l;

    fillMaze();
    addBorders();
    centerWalls();
    getaway();
    addWalls();
}
void Maze::fillMaze(){

    map = new int* [rows];
    for (int i = 0; i < rows; i++) {
        map[i] = new int[columns];
    }

    //Inicializamos toda la tabla
    for (int i = 0; i < rows; i++) {
        for (int j = 0; j < columns; j++) {
            map[i][j] = PASSAGE;
        }
    }
}

Also delete [] map is not enough right ? I'm doing in the destructor of the class:

Maze::~Maze(void)
{
    for (int i = 0; i < rows; ++i)
          delete[] map[i];

        delete[] map;
}
eddie
  • 1,252
  • 3
  • 15
  • 20
  • 3
    Except the missing bad_alloc stuff, it looks correct until now, but any reason you don't use a vector? RAII surely would be useful – deviantfan Oct 13 '15 at 23:23
  • 1
    Apart from it being quite a bad idea to allocate each column separately (use a vector of size `rows * columns` instead and index it with `[i * columns + j]`) why would you even care about not enough memory? What would you do if you don't have enough? Maybe it's better to just let your program die? – Rostislav Oct 13 '15 at 23:25
  • @Rostislav Seriously? One has some data amount where running out of memory is expected, and instead of an easy treatment in the code, you suggest to let the program crash? – deviantfan Oct 13 '15 at 23:26
  • @deviantfan: I see no suggestions for a *crash*. And honestly, in 99% of apps there's no point of handling an out of memory situation. Exit with an error code. – Karoly Horvath Oct 13 '15 at 23:48
  • 1
    @deviantfan A large share of (real) applications have no way of gracefully handling out-of-memory exceptions. That's why I'm saying - just let it die. Thus my question - what would you do if you don't have enough memory? Just exit? Fine. Put your data into vector, put a try block around resize, catch the exception and exit. However, what the heck should be the _maze_ size on which system for this try-catch block to actually matter? You could put a try block in your `main` for that matter. – Rostislav Oct 13 '15 at 23:56
  • @deviantfan If your app _has_ a reasonable way of handling out-of-mem exceptions - I don't have any problems with it. – Rostislav Oct 13 '15 at 23:57
  • There are several ways of dealing with out of memory. The Captain Oates pattern mentioned at http://www.smallmemory.com/ is one that I particularily like. – Jerry Jeremiah Oct 14 '15 at 00:47
  • The problem is that small memory systems are the very special case of application environment. In a modern general purpose operating system one even can't predict how much memory could be allocated (with `brk`, `new`, `malloc` & Co) because of optimistic malloc on the one hand and various fine-grained restrictions of a virtual environment on the other. Don't forget about OOM Killer in the end. So the practical sad but true recipe for a general-purpose application in a general-purpose environment: let it die unexpectedly, and try to minimize consequences. – user3159253 Oct 14 '15 at 01:58
  • 1
    Can i put somebody how to deal with memory exception ? I just want to have an error message and a clean exit and not a segmentation fault. So i also assure is not a code problem, is a memory problem. Thanks in advance. In C i used malloc ( size of) and if it returns null i cant throw the exception. Dont know in C++ in bidimensional arrays. – Eduardo Gutierrez Oct 14 '15 at 07:58
  • int *p = NULL ; p = new int [100]; if (p == NULL) cout << “No enough memory”; – Eduardo Gutierrez Oct 14 '15 at 18:06
  • what is wrong with writing a try-catch block in main and catching 'const std::exception&' and printing out the string returned from why()? Also -- why should your 'maze' need to exist without the matrix being allocated? I'm getting crazy when I see setup code outside the constructor. If new fails, it does not return 0 but throws std::bad_alloc() which derives from std::exception. –  Oct 27 '15 at 20:33
  • @user3159253 Are you really calling an OS with an OOM killer an "modern OS"? –  Oct 27 '15 at 20:44
  • Well, it's a feature, not a bug since it can be disabled :) But mere mortals do prefer use it. – user3159253 Oct 27 '15 at 22:14

1 Answers1

0

This is why you should use std::vector<> as it will handle all the memory management for you correctly.

Issue 1: new never returns NULL (for pedants the normal new as used below). Also note in C++ we use nullptr not NULL for type safety.

 map[i] = new int [columns];
 if (map[i] == NULL)                // This will never be NULL
     cout << “No enough memory”;    // So this will never print anything.

If fillmaze() is called from the constructor. Then if it throws an exception your destructor is never called. This means you have to deal with any allocation failure in place.

    map = new int* [rows];              // could throw.
    for (int i = 0; i < rows; i++) {
        map[i] = new int[columns];      // could throw
    }

So you have to handle and be able to detect what has been allocated and thus what needs to be dallocated.

try
{
    map = nullptr;                      // Init to nullptr to make sure
                                        // you can distinguish between valid state
                                        // and what could have randomly been in memory.

    map = new int* [rows]{nullptr};     // Initialize all elements to nullptr.
                                        // If any throw you don't want your code
                                        // to start releasing random pointers.
                                        // calling delete on nullptr is fine.

    for (int i = 0; i < rows; i++) {
        map[i] = new int[columns]{PASSAGE};
    }
}
catch(...)   // catch all exceptions.
{
    if (map != nullptr) {                   // You should only loop over
                                            // map if it has been allocated.
        for (int i = 0; i < rows; i++) {
            delete [] map[i];
        }
    }
    delete [] map;                         // delete the map.
    throw;                                 // rethrow exception

}

Your destructor is dine.


But you can simplify this by simply using a vector:

 std::vector<std::vector<int>>   maze{std::vector<int>{PASSAGE, columns}, rows};
Martin York
  • 257,169
  • 86
  • 333
  • 562