2D Object Array Memory Issues C++

Question

When my input looks like this:

11

Harry Kate Fred Carol

My adjacency matrix should put Harry at [7][7], Kate at [7][10], Fred at [7][5], and Carol at [7][2]. However, Carol & Kate get inserted at other locations within the adj. matrix as well. I am guessing this has something to do with stack and heap memory, but I am not sure how to find the bug. Using cout statements, there doesn't appear to be any issues.

Code below

#include <iostream>
#include <string>
#include <stdlib.h>
#include <iostream>
#include <sstream>
using namespace std;

class Element {

public:
    string name;
    int weight;
    string color;

    //default constructor
    Element(void) {
        name = "";
        weight = 0;
        color = "white";
    }

    //parameterized constructor
    Element(string first) {
        name = first;
        weight = 0;
        color = "white";
    }

    void setBlack() {
        color = "black";
    }
};

class AdjMatrix {

public:
    int size;
    Element ***adj_matrix;
    AdjMatrix(void) {
        size = 0;
        adj_matrix = NULL;
    }

    AdjMatrix(int n) {
        size = n; //sets the size to n
        adj_matrix = new Element **[size];
        for (int i = 0; i < size; i++) {
            adj_matrix[i] = new Element *[i];
        }
    }

    //Destructor class
    ~AdjMatrix(void) {
        delete adj_matrix;
        adj_matrix = NULL;
    }

    //initialize the array with empty elements
    void initialize_matrix() {
        Element *add_element = new Element("");
        for (int i = 0; i < size; i++) {
            for (int j = 0; j < size; j++)
                adj_matrix[i][j] = add_element;
        }
    }
};


int convertToASCII(string letter)
{
   int x = letter.at(0);
   int index = x - 65;
    return index;
};

int main(int argc, char *argv[]) {

    string table_size;
    cout<<"";
    getline(cin,table_size);
    int size = atoi(table_size.c_str());
    AdjMatrix *myGraph = new AdjMatrix(size);
    myGraph->initialize_matrix();

string line;
getline(cin, line);
while (getline(cin,line))
{
    if (line.empty())
        break;

    else {

        int x = convertToASCII(line);
        stringstream linestream(line);
        string temp;

        while (linestream >> temp) {
            int z = convertToASCII(temp);
            myGraph->adj_matrix[x][z] = new Element(temp);
        }
    }
}

//Print graph
for (int i = 0; i < myGraph->size; i++) {
    for (int j = 0; j < myGraph->size; j++)
        cout<<"["<<i<<"]["<<j<<"]: "<<myGraph->adj_matrix[i][j]->name<<endl;
}

  return 0;
}

when I run the program I receive the following output `[7][2]: Carol [7][3]: [7][4]: [7][5]: Fred [7][6]: [7][7]: Harry [7][8]: [7][9]: [7][10]: Kate` Is this not what you expect? — , Apr 17 '14 at 03:33
also, in `initialize_matrix()` you allocate multiple `new Element("")` and then later on in `main()` you say `myGraph->adj_matrix[x][z] = new Element(temp);` causing some memory leak. — , Apr 17 '14 at 03:39
In your output you will notice that Carol is also at [6][10] and Kate is at [8][0]. How would I otherwise initialize the array? I tried setting each location to NULL, but that crashes the program. — OaklandFanatic, Apr 17 '14 at 05:39

score 1 · Answer 1 · edited May 23 '17 at 12:27

1

Your program has following problem which should be corrected.

 //default constructor
    Element(void)

Constructor should be defined as

 //default constructor
    Element()

You should start using the std::vector for your 2D array. This would eliminate the manual memory management. We should avoid to use raw pointer in modern c++. Please refer the following SO post which describes how to use vector for 2D array.

two dimensional array using vector in cpp

edited May 23 '17 at 12:27

Community

1
1

answered Apr 17 '14 at 03:22

Mantosh Kumar

5,659
3
24
48

I am not allowed to use vectors in the assignment, otherwise I would have because they seem much easier to work with. I will be sure to make the change to the constructor. – OaklandFanatic Apr 17 '14 at 03:24
You also have memory leaks. Look at your destructor -- do you think that a single call to `delete` will remove all of those allocations you made with `new`?. The class also doesn't follow the "rule of three", therefore your code is easily broken with a 3 line main() program. – PaulMcKenzie Apr 17 '14 at 03:36

ooga · Answer 2 · 2014-04-17T03:59:51.537

0

In your AdjMatrix(int) ctor, you are constructing your triangular matrix (if that's what you're going for) incorrectly. Try this:

AdjMatrix(int n) {
    size = n; //sets the size to n
    adj_matrix = new Element **[size];
    for (int i = 0; i < size; i++) {
        adj_matrix[i] = new Element *[i + 1]; // Or size for a square matrix
    }
}

edited Apr 17 '14 at 03:59

answered Apr 17 '14 at 03:10

ooga

15,423
2
20
21

I tried this, but it causes the whole program to crash. Setting the first array to point to another array of the same size, creates a square grid, so that [0][0] is the first location not [0][1] – OaklandFanatic Apr 17 '14 at 03:15
@OaklandFanatic - Please see this answer: http://stackoverflow.com/questions/23067857/c-deep-copy-object/23070443#23070443 In there is a description of two ways of dynamically creating a 2d array. Just replace `float` with `Element`. Note how you create *and* destroy such arrays, something your current code fails to do (the destruction). – PaulMcKenzie Apr 17 '14 at 03:51
@OaklandFanatic If you want a square matrix, then put `size` where I put `i + 1`. Your error is that you have `i` there in your code. – ooga Apr 17 '14 at 03:59
@PaulMcKenzie thanks for the link. I had trouble trying to create a 2D array with a double pointer object but I will try this method again. – OaklandFanatic Apr 17 '14 at 05:41

2D Object Array Memory Issues C++

2 Answers2