5

I'm trying to dynamically allocate (it's not so dynamic as it is right now, but eventually it will be) memory for objects in a very simple C++ program. I'm new to classes and have only recently started playing with C++, leaving C behind. Here's the code:

#include <iostream>
using namespace std;

class Test {
  private:
    int i;
  public:
    Test(int);
    ~Test();
    void print();
};

Test::Test(int ii) { i = ii; }
Test::~Test() { i=0; cout << "deconstructor called...value of i= " << i << endl; }
void Test::print() { cout << "value of i= " << i << endl; }

int main()
{
  Test a(10),*b,*c;
  //a.print(); // this works

  b = new Test(12);
  //b->print(); // this works as well

  for (int i=0; i<2; i++)
    c = new Test(i);

  c->print(); /* this shows that the value of i=1 .. should be 0? */
  c[0].print(); /* as expected (I guess), this prints i=1 as well... [expected because c->print() shows i=1 also */
  c[1].print(); /* shows value of i=0... */

  //delete []c; /* this fails miserably, but `delete c` works, why :( */

}

A lot of my confusion is actually included within comments in the code itself. I'm basically trying to have an array c where each element of the array is an object of itself.

The behavior of the code that I'm getting is described in the comments.

Amit
  • 7,688
  • 17
  • 53
  • 68
  • 1
    Oh wait, darn, I think I see where the mistake is... Almost obviously, it's at this line: `c = new Test(i);` ... now I just need to understand how to fix it... I can't do `c[i] = .. ` hmm – Amit Jul 25 '11 at 03:27
  • I'm possibly more confused by your comments than you are by the program. Anyway, you are leaking memory left right and centre with your program; every call to `new` must be matched by precisely one call to `delete`. Also in your destructor do you intentially set `i` to zero *before* printing it? – Kerrek SB Jul 25 '11 at 03:27
  • I guess the problem is how do I dynamically allocate memory for these objects and simultaneously initialize them? – Amit Jul 25 '11 at 03:28
  • @Kerrek SB: you're definitely right about memory leakage.. and yes I meant to set i to 0 before printing it. – Amit Jul 25 '11 at 03:29
  • Hint: you have to allocate the whole array dynamically, not just its elements. – Beta Jul 25 '11 at 03:30
  • @Kerrek SB: What I'm really trying to accomplish is something on the lines of `c = new Test[2]`, but then I need to understand how to initialize each member object of `c` during its creation. – Amit Jul 25 '11 at 03:30
  • You can't (as far as I know). Why is it so important? – Beta Jul 25 '11 at 03:33
  • ...Come to think of it, there *is* a way, but it's not very general. – Beta Jul 25 '11 at 03:43

6 Answers6

6

Perhaps we should look at the declarations, expanded you have:

Test a(10);
Test *b;
Test *c;

You have defined b and c as pointer-to-Test, but you seem to want c to be an array of pointer-to-test. The declaration for c you intended was likely:

Test **c;

which you would initialize:

c = new Test*[2];

for (int i=0; i<2; i++)
   c[i] = new Test(i);

and which you would access thus:

c[0]->print();
c[1]->print();
Dave
  • 10,964
  • 3
  • 32
  • 54
  • c = new Test*[2]; That was my guess as well, but when I tried this, I got a compiler error. Any idea why? – K Mehta Jul 25 '11 at 03:36
  • How did you have c declared? `Test *c` or `Test **c`? – Dave Jul 25 '11 at 03:41
  • @Kshitij, because `new Test*[2]` returns `Test**` and type of `c` is `Test*`. You should be doing rather `new Test[2]`. – iammilind Jul 25 '11 at 03:42
  • Are you certain? The rules for one-line declarations are often misunderstood, leading to declararions with not enough stars. What error were you getting? – Dave Jul 25 '11 at 03:50
  • @Dave I'll play around with it a little when I have some time, and ask a question on SO with the exact exception if I can't figure it out. Thanks! – K Mehta Jul 25 '11 at 06:12
  • what does the `2` in `c = new Test*[2]` stand for? Size of the array of pointers to Test? – simplename Feb 02 '17 at 05:27
5

There are few serious problems with the given code.

  1. Performing new on *b but missed to delete it.
  2. You are overwriting *c few times in for loop, which will leak memory. Always deallocate resources before allocating a new one from a pointer.
  3. If you are allocating with new/new[]/malloc then you must deallocate the pointer with delete/delete[]/free respectively. The same you are not maintaining with *c (that's why it fails).

Also, apart from learning dynamic allocation one should also be aware of STL containers, which provide a better way of handling dynamic resources. e.g. std::vector.

iammilind
  • 68,093
  • 33
  • 169
  • 336
  • and Mahesh: Thanks. Wasn't aware of the new/delete, new[]/delete[], and malloc/free pairing. That makes sense and clears things up. – Amit Jul 25 '11 at 03:33
0

delete c[]; Only deletes starting element. If you want to delete that array use dz delete c[] in for loop

You failed to allocate memory for c and keep on coding its wrong how can you get a output without allocating memory to a pointer variable?

Eric Hotinger
  • 8,957
  • 5
  • 36
  • 43
0

See according to me , you have allocated memory for *c multiple times as

for (int i=0; i<2; i++)
c = new Test(i);

Have a look on this code, which will make everything clear

for (int i=0; i<2; i++)
{   c = new Test(i);    }       /*see , here the loop goes for i=0; then
                                for i=1; which basically overwrites what c will have
                                 i.e. finally       c = new test(1); */
c->print(); /* works fine , gives value of i=1 */
c[0].print(); /* as expected , this prints i=1 as well... */
c[1].print(); /*clearly, it will give a garbage value */
delete c;

But According to me , it would be more fine to replace

for (int i=0; i<2; i++)
{   c = new Test(i);    }

with

c = new Test(1);    //as the previous code is doing the same in for loop but that was consuming more resources

So if you want output as i=0 and then i=1 then do so-

c = new int(0);
c->print(); /* works fine , gives value of i=0 */
c[0].print(); /* as expected , this prints i=0 as well... */
delete c;

c = new int(1);
c->print(); /* works fine , gives value of i=1 */
c[0].print(); /* as expected , this prints i=1 as well... */
delete c;

The above code is what which will completely satisfy your need.

0
for (int i=0; i<2; i++)
    c = new Test(i);

The above code leaks the memory. c just point to the lastly constructed object in the loop iteration.

c->print(); /* this shows that the value of i=1 .. should be 0?

Here c points to location constructed on new Test(1);. So, the output.

Every new[] should be accompanied with delete[] and new with delete. You cannot intermix both.

Mahesh
  • 34,573
  • 20
  • 89
  • 115
0

That the delete[] doesn't work is perfectly normal: you never allocated c as an array, but as a pointer. You could store the address of an array in a pointer, but that's all. I'm actually wondering why exactly c[1] works, because your for loop just stores repeatedly pointers to newly allocated objects in the same pointer (you are not populating an array!).

Nowhere man
  • 5,007
  • 3
  • 28
  • 44
  • It works, because one can always index on a pointer. It's referencing a possibly unreserved memory address one pointer-width beyond `c`, and only HAPPENS to work for this program, but could also very well fault. – jdmichal Jul 25 '11 at 03:43
  • Yeah, I know about the referencing, I'm merely wondering how it so happens that the memory one object after the pointer seems to be valid. – Nowhere man Jul 25 '11 at 14:34
  • Because it was likely allocated to the executable's heap space by the OS as soon as the program ran. Most OSes will allocate memory in blocks to an executable, meaning that an executable often owns memory that the program hasn't formally allocated. – jdmichal Jul 25 '11 at 15:38