Dynamic array of objects causes crash

[spankboy11]

Hi there, I'm having trouble dealing with dynamic arrays and was wondering if someone could tell me where the problem lies. I'm new to C++.

Basically, in the following code, the dynamic array of Fuzzy objects works fine. But when you uncomment out the bit that actually assigns values to these new objects then the program crashes during runtime.

I'm assuming it's something to do with the memory allocation but I can't work out what. Is this dynamic array procedure something that's not suitable for use with objects? If not, what would be the correct way of going about it?

Thanks very much for any help!

#include <iostream>
#include <fstream>
using std::ifstream;
using std::ofstream;
#include <new>
#include <windows.h>
#include <cstdlib>
#include <conio.h>
#include <cstdio>
#include <time.h>
#include <ctime>

using namespace std;

class Fuzzy{
    public:
        static int n;
        Fuzzy () { n++; };
        ~Fuzzy () { n--; };
        int position[48];
        int score;
    };

int Fuzzy::n = 0;

int main () {
    int i;
    Fuzzy *ent;
  while(1){
      cout << "How many objects? ";
      cin >> i;
      ent = new (nothrow) Fuzzy[i];
      if( ent == 0 ){
          cout << endl << "No memory!";
          }
      /*for( int n = 0; n < Fuzzy::n; n++){
           for( int i = 0; i < 48; i++ ){
                ent[n].position[i] = 24;
                }
           ent[n].score = 567;
           }*/
      cout << ent[0].n << endl;
    }
    system("pause");
    return 0;
}

[Kanoisa]

Hi i've solved the dynamic error problems by using the std::Vector container class. But i believe the memory allocation issue occours when your program loops back around at the end of the while loop and trys to dynamically allocate over the top of what you have there.

Try cleaning up with delete[] ent; first before reallocating or look at what i have done using vectors. the only problem i have encountered with vectors (dont know about your way as i didnt check) is that the static n variable is throwing out the wrong numbers. (unless theres some hidden constructor calls i cant find)

Hope this helps, my edit of your code attached

[sundip]

int main () 
{
    int i;
    Fuzzy **ent;
    while(1)
    {
      cout << "How many objects? ";
      cin >> i;
      *ent = new (nothrow) Fuzzy*[i];
      if( ent == 0 )
      {
          cout << endl << "No memory!";
      }
      for( int n = 0; n < Fuzzy::n; n++){
       ent[n] = new (nothrow) Fuzzy;
           for( int i = 0; i < 48; i++ ){
                ent[n].position[i] = 24;
                }
           ent[n].score = 567;
           }
      cout << ent[0].n << endl;
    }
    // Just deallocate memory also 
    // for( int n = 0; n < Fuzzy::n; n++)
    // {
    //    delete[] ent[n]; 
    //  }
    //  delete[] ent;
    system("pause");
    return 0;
}

It is just like you are allocating memory for a matrix

[Luther von Wulf]

Is this dynamic array procedure something that's not suitable for use with objects?

Dynamic arrays work with objects, the problem is the value of the static variable Fuzzy::n. Destructors are only called if the memory is freed, so if you create an array of 10 in the first iteration of the loop without freeing the array, then 2 in the second, Fuzzy::n never decreases to 0. The initialization will try to populate an array of 12 when there are really only 2 elements. The fix is of course to free the array.

int main () {
    int i;
    Fuzzy *ent;
    while(1){
        cout << "How many objects? ";
        cin >> i;
        ent = new (nothrow) Fuzzy[i];
        if( ent == 0 )
            cout << endl << "No memory!";
        for( int n = 0; n < Fuzzy::n; n++){
            for( int i = 0; i < 48; i++ )
                ent[n].position[i] = 24;
            ent[n].score = 567;
        }
        cout << ent[0].n << endl;
        delete[] ent; // this line was added
    }
}

[mike_2000_17]

Don't use the code from sundip, it is erroneous.

In your original code, there is a memory leak because you loop again and reallocate the memory for a new array on top of the previous one. Since the destructor of all those Fuzzy objects don't get called, the n value keeps on going up and up. So after the first while-loop iteration, that first for-loop with n as bound will corrupt the memory and cause a crash.

Try this code and it works (I tried it, not that I needed to, really..):

#include <iostream>

using namespace std;

class Fuzzy{
    public:
        static int n;
        Fuzzy () { n++; };
        ~Fuzzy () { n--; };
        int position[48];
        int score;
    };

int Fuzzy::n = 0;

int main () {
  Fuzzy* ent;
  for(int iter=0; iter<50; ++iter) {
    ent = new Fuzzy[20];
    for( int n = 0; n < Fuzzy::n; n++){
      for( int i = 0; i < 48; i++ ){
        ent[n].position[i] = 24;
      }
      ent[n].score = 567;
    }
    cout << Fuzzy::n << endl;
    delete[] ent; //delete the array you have.
    cout << Fuzzy::n << endl;
  };
  return 0;
};

The output is 20 0 20 0 20 0 20 0 ... as it should be. If you take out the delete[], it will print 20 20 and it will crash.

[spankboy11]

Thanks for all your help so far guys!

The problem is, I want to keep all the new objects that have been created, so that the value of Fuzzy::n keeps going up as new records are created and for the program to spit out an error only when there is insufficient memory to store any new objects.

With the (nothrow) command I thought that the computer would check to see if there was enough room for Fuzzy objects and only continue if there was. Am I going about this in completely the wrong way?

Thanks again!

[Kanoisa]

For that i think you need to do something like.

char
//add new objects
Fuzzy *temp = new Fuzzy[Fuzzy::n+i];//where i is your user entered value
//loop through ent[0]->ent[Fuzzy::n-1] and assign to temp;

//create new objects to add to the end of the temp array;

//assign temp to ent;

I think this is the gist of what you want to do but might not be quite right just yet. from this i think you can get what you want. but may i suggest vectors and they contain all the functionality to do this internally which saves you allot of time. considering ent as a vector you could do this.

std::vector<Fuzzy*> ent; //creates ent as a vector(dynamic array) of Fuzzy instance pointers

//get user input into i

for(int n = 0; n<i; n++)
{
  Fuzzy *temp; //temp pointer
  temp = new Fuzzy; //give temp an instance
  ent.push_back(temp); //store address of new instance in ent
}

That seems the easiest system to me but if you want to keep your code as id thats fine, its just a sugegstion

[spankboy11]

Thanks for your suggestion Kanoisa - I'll give it a go and report back.

To be honest, I was hoping to avoid learning how to use vectors as it's yet another thing to confuse me in this baffling world of C++! But I suppose I'll have to learn about them sooner or later...

[Kanoisa]

Vectors are very simple i thought it was strange at first vut its simple ill explain a little for you.

//create a vector or array
vector<Type> varName; //creates an array name (empty at first)
Type varName[vecSize];//creates a constant sized array

//append an item;
varName.push_back(Type Item); //appends an item of the vector type to the vector

//to loop through an array you need the size, its easy here
for(int n = 0; n<varName.Size(); n++) //.Size() returns an integer telling you how many items are in the vector
{
  //can use indicies like an array
  varName[n] = someThing;
}

//if you know the size you want the vector at first you can save allocation time attributed to push_back

varName.Resize(SizeIWant);

hope that shows you the basics of a vector if you get stuck just ask and remember vectors require thier header #include<vector> and you will want either using namespace std or using std::vector or similar.

Trust me for like months i refused to use stl cause i thoguht well why when i can do it myself but once you get used to it you can use so much with a common interface ie

vector<int> vInt; //vector of ints
llist<int> llint; //double linked list of ints

//examples
vInt.push_back(3);
llint.push_back(4);

they all work the same way and the storage type is transparent its exellent

please forgive typos i typed this in a rush

[spankboy11]

Thanks for your very helpful post Kanoisa - I went back and looked at the rewrite of my code that you posted above, storing the new objects in a vector and I think I've got vectors all worked out now. And yes, they're pretty helpful!

I was a bit uncomfortable about the Fuzzy::n being totally wrong, cos I didn't like the fact that all these destructors were being called and me not knowing why, so I fiddled around a bit and came up with this code:

int main () 
{
    vector<Fuzzy *>ent;
    int number = 0;
    while( 1 ){
        cout << "How many new elements to add? ";
        cin >> number;
        for( int n = 0; n < number; n++ ){
            Fuzzy *obj = new Fuzzy; // create pointer to new Fuzzy
            ent.push_back( obj );   // store pointer in ent
            }
        cout << "size of ent: " << ent.size() << endl;
        cout << "capacity of ent: " << ent.capacity() << endl;
        cout << "max_size of ent: " << ent.max_size() << endl;
        cout << "Fuzzy::n: " << Fuzzy::n << endl;
        cout << "Value stored in ent.size - 1: " << ent[ent.size() - 1] << endl;
        *(ent[ent.size() - 1]).score = 8; // get value of Fuzzy.score stored at ent[i] ???
        }

    return 0;
}

... which (mostly) works fine and Fuzzy::n returns the right answer. BUT I unfortunately don't know how to dereference the address of the object pointed at by the value stored in the vector! The line that tries to assign 8 to Fuzzy::score just returns a compiler error that score hasn't been declared.

I think I'm nearly there, but just a little bit more advice to help me limp over the finishing line would be gratefully received!

[spankboy11]

I think I have it.. it's the -> operator, right?

[Kanoisa]

Ok thats a nice simple one.

basically its not happy with the *(ent[..]). notation (im not the best at using that so im not exactly sure why) however just swap to the following style to access elements and it defrences correctly. Also added a wee score printout for you.

ent[ent.size()-1]->score = 8;
cout <<"score = "<<ent[ent.size()-1]->score<<endl;

hope this solves that issue for you and happy coding

edit: didnt see that last post, so the simple answer was yes :P