Solved

Is this a justified explicit call of a destructor ?

Posted on 2003-12-07
19
309 Views
Last Modified: 2010-04-02
Ah hello.

I would appreciate it if you could please examine the following code, with particular attention being paid to

Matrix& Matrix::operator=(const Matrix& rhs)

Have I coded this correctly, and is there any need for it to be virtual ?

Cheers.

#include <iostream>

using std::cout;
using std::endl;
using std::ostream;

class Matrix
{
public:
      Matrix(int r, int c) : nr(r), nc(c), data(memory_allocate(r, c)) { intialise(); }
      Matrix(const Matrix& m);
      virtual ~Matrix() = 0;
      double& operator() (int i, int j){ return data[i][j]; }
      friend ostream& operator << (ostream& os, const Matrix& rhs);
      virtual ostream& print(ostream& os) const;
      virtual Matrix& operator=(const Matrix& rhs);
protected:
      int nr;  // Number of rows
      int nc;  // Number of cols
      double** data;
      double** memory_allocate(int r, int c);
      void intialise();
};

Matrix::Matrix(const Matrix& m) : nr(m.nr), nc(m.nc), data(memory_allocate(m.nr, m.nc))
{
      for (int i = 0; i < nr; ++i) {
            for (int j = 0; j < nc; ++j) {
                  data[i][j] = m.data[i][j];
            }
      }
}

Matrix::~Matrix() {
      for (int i = 0; i < nr; i++) {
            delete[] data[i];
      }
      delete[] data;
}

double** Matrix::memory_allocate(int r, int c)
{
      double** temp;
      temp = new double*[r];
      for (int i = 0; i < r; i++) {
            temp[i] = new double[c];
      }
      return temp;
}

ostream& operator << (ostream& os, const Matrix& rhs)
{
      return rhs.print(os);
}

ostream& Matrix::print(ostream& os) const
{
      for (int r = 0; r < nr; r++) {
            for (int c =0; c < nc; c++) {
                  os << data[r][c] << "\t";
            }
            os << "\n";
      }
      return os;
}

Matrix&      Matrix::operator=(const Matrix& rhs)
{
      if (this != &rhs) {
            this->~Matrix();
            memory_allocate(rhs.nr, rhs.nc);
            this->intialise();
      }
      return *this;
}


void Matrix::intialise()
{
      for (int r = 0; r < nr; r++) {
            for (int c =0; c < nc; c++) {
                  data[r][c] = 0;
            }
      }
}

class Vector : public Matrix
{
public:
      Vector(int nr, int nc) : Matrix(nr, nc) {}
      Vector(const Vector& v) : Matrix(v) {}
      virtual ~Vector() = 0 {};
      virtual double& operator[] (int i) = 0;
      virtual double operator[] (int i) const = 0;
      virtual ostream& print(ostream& os) const;
};

ostream& Vector::print(ostream& os) const
{
      return (Matrix::print(os));
}

// Assuming one row, varying number of columns, eg xxxxx is a row vector of size 5
class RVector : public Vector
{
public:
      RVector(int nc) : Vector(1, nc) {}
      ~RVector() {}
      // Called for assignment/retrieval on a NONE CONSTANT object only
      virtual double& operator[](int i) { return data[0][i]; }
      // Called for assignment/retreival on a CONSTANT object only
      virtual double operator[](int i) const { return data[0][i]; }
};

int main()
{
      RVector r(10);
      RVector s(10);

      cout << s << endl;

      for (int i = 0; i < 10; i++) {
            s[i] = 10;
      }

      s = r;

      //r.~RVector();

      cout << s << endl;

      return 0;
}

 
0
Comment
Question by:mrwad99
  • 11
  • 5
  • 3
19 Comments
 
LVL 30

Expert Comment

by:Axter
ID: 9894021
Calling a virtual destructor from within a base class function will lead to undefined behavior.

The destructor will delete any child class objects.
0
 
LVL 30

Expert Comment

by:Axter
ID: 9894024
Example:
class foo
{
public:
      foo(){intialise();}
      virtual ~foo() = 0;
      virtual foo& operator=(const foo& rhs);
protected:
      void intialise()
      {
            data = new char[33];
            strcpy(data, "Hello World");
      }
      char *data;
};

foo::~foo() {
    delete[] data;
}

foo&     foo::operator=(const foo& rhs)
{
    if (this != &rhs) {
         this->~foo();
         this->intialise();
    }
    return *this;
}

class SomeObj
{
public:
      SomeObj(){ cout << "ctor SomeObj" << endl;
      }
      ~SomeObj(){ cout << "dtor SomeObj" << endl;
      }
      int x;
};

class child_foo : public foo
{
public:
      child_foo(){ My_SomeObj.x = 99;
      }
      ~child_foo(){
      }
      void SomeFunc()
      {
            cout << My_SomeObj.x << endl;
      }
private:
      SomeObj My_SomeObj;
};

int main(int argc, char* argv[])
{
      child_foo f1;
      child_foo f2;
      f1 = f2;

      f1.SomeFunc(); //Undefined behavior
      
      system("pause");
      return 0;
}

0
 
LVL 30

Accepted Solution

by:
Axter earned 20 total points
ID: 9894041
The above code will produce the following results:
ctor SomeObj
ctor SomeObj
dtor SomeObj
99
Press any key to continue . . .


The last line may come out to be 99, or it may just crash, or it may display garbage.  The result is undefine.
As you can see, there are two calls to the constructor, and one call to a destructor.
So when f1.SomeFunc() is called, this function ends up using an object that has been deleted.
The My_SomeObj has been deleted before the life of the child_foo object has ended.
0
 
LVL 13

Expert Comment

by:SteH
ID: 9895092
In your code for the operator= you only need the deallocation of the memory not of the entire object. So replace the call to the destructor with the code inside the destructor:

Matrix&     Matrix::operator=(const Matrix& rhs)
{
     if (this != &rhs) {
          // attention can the memory be reused?
          for (int i = 0; i < nr; i++) {
               delete[] data[i];
          }
          delete[] data;
          memory_allocate(rhs.nr, rhs.nc);
          this->intialise();
     }
     return *this;
}

and depending on the dimensions of the matrix / vector you are mainly using I expect to see identical values for nr and nc often. In that case I would suggest to compare for equality before reallocation memory of the same size.

Do you need this-> in this->initialise()?
Why are you not copying the data in the matrix from rhs to this? This is definitely needed unless the class is pure virtual.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9895302
Thanks for those comments, will get back to you all this evening/tomorrow due to time constraints.  

Thanks thus far :)
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9907023
Axter,

I had to wait until I had a compiler before I could come back about that example code you posted.  Now I am amazed that the code does output 99; I changed it slightly so that x of SomeObj was created on the heap and the output is still the same, even with the explicit call to 'delete x' within the destructor.

?!?!?

Why is this ?

SteH,

Yeah you are right.  I was thinking of not duplicating code, so explicitly called the destructor to clear out 'data' forgetting that the *whole* object would be deleted as a consequence.  And I am aware that I don't need the explicit 'this->'; I just wanted to make it crystal clear whilst coding it.  Also the case where I dont need to reallocate if the new dimensions are the same as the old ones.

I did however notice a bug in my own code regarding the initialise function; using nr and nc will use the old values for these variables, not the new ones, hence if the new values are less than the old ones,  the loop will still attempt to access values that are 'out of bounds'.  Ah well, at least I spotted it myself !  Can't award myself points though ;)

Now, a related question to which I will award a further 25 points:  I am  going through old University exams, the code in the original question was what I have accomplished so far.  But, the final question is still puzzling me:

(Assume that the operator= is not yet coded)
-------------------------------------


"Assume the following code:

RVector v1(5), v2(5);

v1 = v2;
v1.~RVector()
cout << V2[0];

What happens ?  To resolve the problem, add one function in the Matrix, Vector and RVector classes, and define it in all three places".

--------------------------------------

Now the problem is obviously the lack of operator= meaning that the same memory is being pointed to by v1 and v2.  When v1 is deleted, v2 points to empty memory.  Fine.  But what I do not get is the need to define the operator= in all *three* classes.  

Now, = can only really be carried out when both operands are of equal 'size'; e.g. both 5x5 matrices or both row vectors of size 5 etc.  So what my implementation is missing straight away is some sort of test for this.  Return *this if not the same methinks.  Correct ?  Now I have put the implementation of operator= in Matrix, the base class, so it can be inherited.  Indeed it is, as s = r shows.  So why the need for three definitions of this function ?!?  If I make the matrix one virtual, it means I have got to defer the implementation until RVector.  Is this what the examiners were looking for do you think ?  (Can't get the answers, have left University and I don't think the answers were released anyway.  Grrr)

- Confused.

Many thanks thus far.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9907046
Grrrr.

>> If I make the matrix one virtual, it means I have got to defer the implementation

Should be

If I make the matrix one *pure* virtual, it means I have got to defer the implementation
0
 
LVL 13

Expert Comment

by:SteH
ID: 9910407
No: a pure virtual class can have functions implemented. Only no instance of these class can be created. And the pure virtual function definitly needs to be implemented later.

The compiler generated copy contructor is, to my knowlegde, not copying the address but it is creating a new object and all data members are copied as if they where primitive datatypes. So there is no problem to use this operator for data members like int, double, etc. Only your double** will receive a copy ofthe address. When deleting v1, v2 will contain usefull data in c and r but not in data.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9920020
>> If I make the matrix one *pure* virtual, it means I have got to defer the implementation

Did I really say that ?  OMG I am losing it !

SteH, what you have said is fine, data will be invalid when the first object is destroyed, since there was no operator= defined to allocate new memory.  So my code stays the same.

But what I am still unsure about is the question asking to add the function to all *three* classes and define it in all *three* places.  Any ideas ?
0
How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

 
LVL 19

Author Comment

by:mrwad99
ID: 9920027
>> And the pure virtual function definitly needs to be implemented later.

This has puzzled me.  What do you mean ?
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9920029
<points increased to 65>
0
 
LVL 13

Expert Comment

by:SteH
ID: 9920478
A derived class not implemeting the pure virtual function of the base class is an abstract class as well. So to get a class that is not abstract you need to define all pure virtual function of base class(es). You need to implement the function for the first instanciable class and the other virtual functions can't be called. So why bother about defining them?

But coming back to your case: Only the Matrix class has data members and they are protected. In that case I would define a operator= for that class to allocate new memory for the new objects and copy its content. Since neither Vector nor RVector define data members the default assignment operator is sufficient. Nothing to do here except calling the base class assignment operator.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9920561
My point exactly.  As I said before, I was confused at *the exam question* asking for the definition of all three.

In fact, I have been thinking.  In the case of

lhs = rhs

the only time that an assignment can be carried out is if both lhs and rhs are *of exactly the same size*.  Now, I am thinking, a Vector object can be either a COLUMN vector, see (1), or a ROW vector (2).

1) Column vector, size 5

x
x
x
x
x

2) Row vector, size 5

xxxxx

So maybe the exam question is asking for the RVector class to first check the dimensions; if assignment can be carried out then the Matrix::operator= is called.  

Same principle for what could be CVector too.

However, if this is the case, and lhs != rhs in dimensions, how would I alert this fact to the user ?  Print an error message and return *lhs, or just use Asserts ?  What do you think ?
0
 
LVL 13

Expert Comment

by:SteH
ID: 9920739
The assignment can be done in any case. The new dimension of the lhs is the same of the rhs after the assignment and all the values are copied as well. Thats what the assignment is used for. For an operator+ this is something different. Or if you want to multiply those vectors with a matrix. But still here I don't see a need for an operator= in the inherited classes. You could perhaps try it in a simple prog.

And if new memory needs to be allocated because dimension size have changed it should be the responsibility of Matrix::operator= since the data members to be changed are members of class Matrix.

0
 
LVL 19

Author Comment

by:mrwad99
ID: 9920778
Yeah again you are right.  It seems to me that the operator= should not allocate *more* memory, it shoud allocate the same amount of memory, just *new* memory.  I mean, the purpose of assigning as I see it NOW is to copy values, not increase or decrease memory usage.  Assigning a 5x5 matrix to a 4x4 matrix should not result in the 4x4 matrix being increased in size; it should fail.  

A 5x5 matrix with all values set to 1 assigned to a 5x5 matrix with all values set to 2 should simply reallocate the same amount of memory and assign all of the 2s to 1s.

Is this how you would interperet operator= ?

One thing is for sure though, new memory needs to be allocated, which is what the root of the exam question was getting at.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9920819
Grrrr this is getting even more confusing now.

Do I need to allocate more memory in operator= though ?

RVector r(10);
RVector s(8);

creates separate memory for both objects.  Surely therefore, all that is needed in operator= is to assign the values stored in 'data' of the lhs to 'data' of the rhs.  Why do I need to create or delete (more) memory ?
0
 
LVL 13

Assisted Solution

by:SteH
SteH earned 45 total points
ID: 9921012
In the case where both sides have the same dimension you can just copy data. Only in the case when the sizes are different the old memory has to be released and newly allocated. In principle you could reuse the memory when you assign a smaller sized array to a bigger one. But how do you keep track of the amount of allocated memory?

If you lhs is a 5x5 matrix and want to assign it to a 3x3 matrix you could use only the first three double** and copy the values into their double*. For the latter this won't be problematic. delete[] will remove all memory allocated. But how do you later access the last 2 double**'s?

>> Assigning a 5x5 matrix to a 4x4 matrix should not result in the 4x4 matrix being increased in size; it should fail.  
Why? From the point of view of the variable it is no problem. The name of the var can be reused. It is only not safe anymore if you have something like a 3vector and want toassing an RVector(4) to it.

If you want to do matrix multiplications or vector and matrix multiplications you have to take care about dimension sizes but not before. The class RVector for example makes no assumption on the dimension of the vector. Why should you limit the use of instances?
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9921215
OK.  I think this can be closed now.  

But first, (and finally), about the

".....What happens ?  To resolve the problem, add one function in the Matrix, Vector and RVector classes, and define it in all three places"

question: we have established that there is no real need to add it to the derived classes; doing so would obviously be overkill.  I thought that the examiners could have wanted

RVector& RVector::operator=(const RVector& rhs)
{
      return Matrix::operator=(rhs);
}

but of course this cannot compile: returning a Matrix where a RVector is expected.  I hence cannot see where we can get three definitions from for this function.  At all.

Final suggestions ?
0
 
LVL 19

Author Comment

by:mrwad99
ID: 9937456
Does not look like more suggestions are coming in, so I will close this.

I have split the original 40 points 20:20; the 25 for answering the additional question go to SteH as promised.

Thanks all !
0

Featured Post

Maximize Your Threat Intelligence Reporting

Reporting is one of the most important and least talked about aspects of a world-class threat intelligence program. Here’s how to do it right.

Join & Write a Comment

Often, when implementing a feature, you won't know how certain events should be handled at the point where they occur and you'd rather defer to the user of your function or class. For example, a XML parser will extract a tag from the source code, wh…
Introduction This article is a continuation of the C/C++ Visual Studio Express debugger series. Part 1 provided a quick start guide in using the debugger. Part 2 focused on additional topics in breakpoints. As your assignments become a little more …
The goal of the tutorial is to teach the user how to use functions in C++. The video will cover how to define functions, how to call functions and how to create functions prototypes. Microsoft Visual C++ 2010 Express will be used as a text editor an…
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…

757 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

19 Experts available now in Live!

Get 1:1 Help Now