-2

I have a variable that is an array on the heap. I declared this array on the heap using and an integer value that i set times the sizeOf the object. I am having trouble in my code that when I set my object to null, the integer used to create space for it is set to 0.

int numOfNodes;
int numOfEdges;

FILE *file;
file = fopen("input.txt", "r");
fscanf(file, "%d", &numOfNodes); //numOfNodes value is read in correct here             
fscanf(file, "%d", &numOfEdges);//numOfEdges value is read in correct here

ELEMENT **T = (ELEMENT**) new ELEMENT()//setting space on the heap for an array of linked lists 


for(int i=0; i<numOfNodes; i++){//This line is supposed to initialize all the Linked List inside the array to NULL. However on the second iteration the value for int numOfNodes changes to 0.
T[i]=NULL}


Not sure whats going on in that the array of Linked Lists and the int numOfNodes have nothing to do with each other beyond that the numOfNodes is used as a number to allocate an appropriate amount of space on the heap.

EDIT:Used new instead of malloc but still running into the same problem

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
JokimNoah
  • 35
  • 2
  • 1
    Since this is c++ don’t use malloc, use new. And you have a double pointer but you’re not allocating space for pointers, you’re allocating for the structs themselves. This would not work if you used `new` and you would get a proper error message. – Sami Kuhmonen Apr 13 '19 at 06:11
  • 1
    ELEMENT * as the result of the malloc would be sufficient. It is an array of struct ELEMENT so a simple pointer would be sufficient – NGI Apr 13 '19 at 06:13
  • 1
    With your simple pointer you have to initialize all fields of the ELEMENT structure one by one – NGI Apr 13 '19 at 06:16
  • 1
    https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Yunnosch Apr 13 '19 at 06:23
  • 1
    It would be much simpler to just use a `std::array` or `std::vector`. – Jesper Juhl Apr 13 '19 at 09:06

1 Answers1

-1
int numOfNodes= 0; // good practice: always initialize
int numOfEdges= 0; // ditto

FILE* file= fopen("input.txt", "r"); // ditto
// you should check fscanf return value
fscanf(file, "%d", &numOfNodes); //numOfNodes value is read in correct here             
fscanf(file, "%d", &numOfEdges); //numOfEdges value is read in correct here

You should also check that those numbers are > 0.

This is bad, don't do this following:

ELEMENT **T = (ELEMENT**) new ELEMENT()

You're asking memory for just one element, and the memory you're getting is ELEMENT*. By forcing the cast to ELEMENT**, you are not only doing something wrong, but telling the compiler 'trust me, I know what I'm doing'. As a general rule in C++, if you're casting, you're doing something bad. There usually is a better way. And don't use old style cast, prefer reinterpret_cast, static_cast, dynamic_cast and const_cast.

If what you want is an array of ELEMENT*, then you should ask for it:

typedef ELEMENT* pElement;
ELEMENT** T= new pElement[numOfNodes]; // delete with delete[] T;
// or:
ELEMENT** T= malloc(sizeof(ELEMENT*) * numOfNodes); // delete with free(T);

Now you can do:

for (int i= 0; i < numOfNodes; i++) {
  // This line is supposed to initialize all the Linked List inside the array to NULL.
  // However on the second iteration the value for int numOfNodes changes to 0.
  T[i]= NULL; // nullptr on C++11
}

In modern c++ I think the for is not needed if you do:

ELEMENT** T= new pElement[numOfNodes]{}; // delete with delete[] T;

Also you could use auto:

auto T= new pElement[numOfNodes]{};

Note that you're not only removing explicit casts but also implicit casts.

You could also have used in C++

std::vector, which grows as needed
std::vector<ELEMENT*> T(numOfNodes, NULL);

Passing (numOfNodes, NULL) as arguments allocates numOfNodes elements initialized with NULL. Even while std::vector lives on the stack the memory for the elements lives on the heap. You would access it the usual way:

T[10]= new ELEMENT();

Even better, in modern C++:

std::vector<unique_ptr<ELEMENT> > T(numOfNodes);

will construct a vector of smart pointers that will call delete whenever you set them to other value

T[10].reset(new ELEMENT(1)); // will set the pointer to a new element initialized with 1
T[10].reset(new ELEMENT(2)); // will set the pointer to a another new element

initialized with 2 and delete properly the old ELEMENT initialized with 1

T[10]->doSomething();

will work as usual.

user207421
  • 305,947
  • 44
  • 307
  • 483
Mirko
  • 1,043
  • 6
  • 12