Is this a justified explicit call of a destructor ?

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;
}

 
LVL 19
mrwad99Asked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

AxterCommented:
Calling a virtual destructor from within a base class function will lead to undefined behavior.

The destructor will delete any child class objects.
0
AxterCommented:
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
AxterCommented:
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

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
Cloud Class® Course: SQL Server Core 2016

This course will introduce you to SQL Server Core 2016, as well as teach you about SSMS, data tools, installation, server configuration, using Management Studio, and writing and executing queries.

SteHCommented:
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
mrwad99Author Commented:
Thanks for those comments, will get back to you all this evening/tomorrow due to time constraints.  

Thanks thus far :)
0
mrwad99Author Commented:
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
mrwad99Author Commented:
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
SteHCommented:
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
mrwad99Author Commented:
>> 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
mrwad99Author Commented:
>> And the pure virtual function definitly needs to be implemented later.

This has puzzled me.  What do you mean ?
0
mrwad99Author Commented:
<points increased to 65>
0
SteHCommented:
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
mrwad99Author Commented:
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
SteHCommented:
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
mrwad99Author Commented:
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
mrwad99Author Commented:
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
SteHCommented:
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
mrwad99Author Commented:
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
mrwad99Author Commented:
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
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
C++

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.