0

I have a grid class as follows. Using it in my program works fine until the main() function returns then there is an error message and the program crashes due to an uncaught exception. If I comment out the destructor the class works just fine. What would be the correct way to implement this destructor?

If I just delete[] grid I assume that the arrays within it are not deallocated.

Exact error: Unhandled exception at 0x000869F5 in Battleship.exe: 0xC0000005: Access violation writing location 0xDDDDDDDD.

class Grid
{
private:

    int numRows;
    int numCols;
    char** grid; // array of arrays / pointer to pointer to char

public:

    /*****************************************************************
    Constructor()
    *****************************************************************/
    Grid() : numRows(0), numCols(0)
    {
    }

    /*****************************************************************
    Constructor(int, int)
    *****************************************************************/
    Grid(int numRows, int numCols) : numRows(numRows), numCols(numCols)
    {
        grid = new char*[numRows];
        for (int arr = 0; arr < numRows; ++arr) {
            grid[arr] = new char[numCols];
        }
    }

    /*****************************************************************
    Destructor NEEDS MAJOR EDIT AS IT IS CAUSING THE PROGRAM TO CRASH
    *****************************************************************/
    ~Grid()
    {
        for (int i = 0; i < numRows; ++i)
        {
            delete[] grid[i]; //delete all subarrays of grid
        }
        delete[] grid; //delete grid
    }
}
akashchandrakar
  • 2,009
  • 3
  • 23
  • 47
j_v_wow_d
  • 511
  • 2
  • 11
  • 2
    Probably a [rule of three](http://en.cppreference.com/w/cpp/language/rule_of_three) violation. – Bill Lynch Dec 03 '14 at 03:50
  • 4
    The correct way is to use a `std::vector>` (possibly a `std::vector>`), and to remove the destructor – quantdev Dec 03 '14 at 03:50
  • 5
    Your default constructor should be setting `grid` to `nullptr`, and you should check for that in the destructor. Moreover, you need to implement a copy constructor and copy assignment operator at the bare minimum. To avoid all this trouble, use `vector> grid` – Praetorian Dec 03 '14 at 03:52
  • 1
    At a bare minimum, don't implement anything at all and use the standard library. –  Dec 03 '14 at 03:53
  • @remyabel are you talking about a standard library for a grid class because I could not find one. – j_v_wow_d Dec 03 '14 at 04:02
  • @Praetorian I thought about using vector but I am just learning c++. This is the first code I've written so I wanted to use arrays since I don't intend to resize so that i could try to understand the pointer and destructor and deallocation stuff. What happens if I delete[] a null pointer? – j_v_wow_d Dec 03 '14 at 04:05
  • 2
    @j_v_wow_d Using the facilities available in the standard library is something you should be doing as much as possible while learning C++. Manual memory management is an advanced topic that you should attempt after gaining some proficiency with the language. `delete[]` on a `nullptr` is a NOP, but the problem with your code is `grid` is uninitialized in the default constructor, so it may not be `nullptr` and you'll run into undefined behavior in the destructor. – Praetorian Dec 03 '14 at 04:27
  • Also, you don't necessarily have to set `grid` to `nullptr` in the default constructor, and check for that in the destructor as I said earlier. Delegating to the other constructor would work too. `Grid() : Grid(0, 0) {}`. In any case, you'll need to follow the [Rule of Three](http://stackoverflow.com/q/4172722/241631). – Praetorian Dec 03 '14 at 04:29
  • @Praetorian does it make a difference that the only instance of this in my code is Grid grid(10,10); Even though you are correct on the rule of three, how does that come into play in this specific case since grid() is never called nor is there a copy or assignment? Thanks for the help. – j_v_wow_d Dec 03 '14 at 04:34
  • @Praetorian My apologies. My last comment was incorrect. It is instantiated only once with Grid grid(10,10) but that is done inside another class. When I do it directly inside main it deallocates fine, but when instantiated inside the other class, there are problems with ~Grid() – j_v_wow_d Dec 03 '14 at 04:48
  • If you never copy or assign to a `Grid` instance you'll obviously not need those special member functions defined. But the problem is, as you've discovered, you may accidentally do so, or a class containing a `Grid` data member may be copied, which results in the `Grid` being copied and you have UB in no time. – Praetorian Dec 03 '14 at 04:52
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/66085/discussion-between-j-v-wow-d-and-praetorian). – j_v_wow_d Dec 03 '14 at 04:57

0 Answers0