Destructors And Operator Overloading

Discussion in 'C++' started by רמי, Apr 10, 2008.

  1. Hey,

    I've created a small class that overrides the "+" operator, and
    defines a destructor:

    private:
    T *data;
    int size;
    public:
    myClass(int s);
    myClass<T> operator+(const myClass<T>& vec);
    ~myClass();

    The implementation of the operator is:
    myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    {
    int i;
    myClass<T> res(cls.size);

    for (i=0;i<cls.size;i++)
    {
    res.data = data;
    }

    return res;
    }

    Don't judge the code as its just a pseudo of the real one that.

    The destructor is:

    myClass<T>::~myClass()
    {
    delete [] data;
    }

    The problem is, that after the last line of the operator+ - "return
    res"
    I don't know why, but the destructor is called for "res",
    Which means that the object returned, is already cleaned!!!

    What am I missing\doing wrong?

    Thanks ahead

    --sternr
     
    רמי, Apr 10, 2008
    #1
    1. Advertising

  2. רמי

    Barry Guest

    רמי wrote:
    > Hey,
    >
    > I've created a small class that overrides the "+" operator, and
    > defines a destructor:
    >
    > private:
    > T *data;
    > int size;
    > public:
    > myClass(int s);
    > myClass<T> operator+(const myClass<T>& vec);
    > ~myClass();
    >
    > The implementation of the operator is:
    > myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    > {
    > int i;
    > myClass<T> res(cls.size);
    >
    > for (i=0;i<cls.size;i++)
    > {
    > res.data = data;
    > }
    >
    > return res;
    > }
    >
    > Don't judge the code as its just a pseudo of the real one that.
    >
    > The destructor is:
    >
    > myClass<T>::~myClass()
    > {
    > delete [] data;
    > }
    >
    > The problem is, that after the last line of the operator+ - "return
    > res"
    > I don't know why, but the destructor is called for "res",
    > Which means that the object returned, is already cleaned!!!
    >


    a temporary instance of myClass<T> is contructed via "Copy Constructor"
    to copy "res" you just returned. At the end of the function block, "res"
    is destructed.

    But RVO/NRVO ([Named] Return Value optimization) technique is used to
    avoid the copy of for the temp variable.

    you can google it.
     
    Barry, Apr 10, 2008
    #2
    1. Advertising

  3. רמי

    Guest

    On 10 Apr., 10:41, רמי <> wrote:
    > Hey,
    >
    > I've created a small class that overrides the "+" operator, and
    > defines a destructor:
    >
    > private:
    > T *data;
    > int size;
    > public:
    > myClass(int s);
    > myClass<T> operator+(const myClass<T>& vec);
    > ~myClass();
    >
    > The implementation of the operator is:
    > myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    > {
    >                 int i;
    >         myClass<T> res(cls.size);
    >
    >         for (i=0;i<cls.size;i++)
    >         {
    >               res.data = data;
    >         }
    >
    >         return res;
    >
    > }
    >
    > Don't judge the code as its just a pseudo of the real one that.
    >
    > The destructor is:
    >
    > myClass<T>::~myClass()
    > {
    >               delete [] data;
    >
    > }
    >
    > The problem is, that after the last line of the operator+ - "return
    > res"
    > I don't know why, but the destructor is called for "res",
    > Which means that the object returned, is already cleaned!!!


    That is only to be expected: You declare a temporary object called
    "res" which will be filled with the concatenated sequence of elements.
    When the operator+ returns, this variable will be copied to the
    variable that holds the return value in the calling function. After
    this copy, "res" will be destroyed (unless your compiler performs some
    kind of return value optimization). Thus your class needs a copy
    constructor, so that the copy that is seen by the caller of operator+
    receives a proper copy of "res".

    BTW, your operator+ should have the following signature:
    _const_ myClass<T> operator+(const myClass<T>& vec) _const_;
    The first additional const ensures that the following code will be
    prohibited:
    myClass<int> a, b, c;
    (a + b) = c;
    The second const states that your operator+ doesn't change anything
    for the object it is invoked on. This allows you to add const objects:
    const myClass<int> a, b;
    a + b;

    Regards,
    Stuart
     
    , Apr 10, 2008
    #3
  4. רמי

    Barry Guest

    wrote:
    > On 10 Apr., 10:41, רמי <> wrote:
    > BTW, your operator+ should have the following signature:
    > _const_ myClass<T> operator+(const myClass<T>& vec) _const_;
    > The first additional const ensures that the following code will be
    > prohibited:
    > myClass<int> a, b, c;
    > (a + b) = c;
    > The second const states that your operator+ doesn't change anything
    > for the object it is invoked on. This allows you to add const objects:
    > const myClass<int> a, b;
    > a + b;


    I think the second case depends.

    maybe we also want
    a + b + c;
     
    Barry, Apr 10, 2008
    #4
  5. רמי

    Guest

    On 10 Apr., 10:41, רמי <> wrote:
    [..]
    > The implementation of the operator is:
    > myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    > {
    > int i;
    > myClass<T> res(cls.size);
    >
    > for (i=0;i<cls.size;i++)
    > {
    > res.data = data;
    > }
    >
    > return res;
    >
    > }
    >

    [..]
    > The problem is, that after the last line of the operator+ - "return
    > res"
    > I don't know why, but the destructor is called for "res",
    > Which means that the object returned, is already cleaned!!!
    >
    > What am I missing\doing wrong?


    Your implementation creates a local variable res that is destroyed
    when leaving the function body, hence the destructor call.
    "return res" provides a copy of res to the caller.

    However, in the member "data", a copy of the pointer is provided that
    is used to store your array in local variable. When your destructor
    frees the memory, the pointer in the returned object becomes invalid.
    You have two options:
    1. provide a copy constructor that does a "deep copy". I.e. allocates
    a new buffer and copies the memory content pointed to.
    2. avoid T*, use a std::vector<T> instead. This will do all memory
    allocation issues transparently for you, no need for memory
    deallocation in a destructor, no need to write a copy constructor
    yourself.

    The solution 2. is what I would prefer. Your adapted class may look
    like this:

    template <class T>
    class myClass {
    private:
    std::vector<T> data;
    /* int size; */ // obsolete, use data.size() if you have to.

    public:
    myClass( int s );
    myClass<T> operator+( const myClass<T>& vec );
    /* ~myClass(); */ // obsolete, default destructor handles data
    member
    };

    If "data" is *the* central member of your class, to make handling more
    convenient I'd also consider inheritance:

    template <class T>
    class myClass : private std::vector<T> {
    ...
    };

    The implementation of your + operator will become:

    template <class T>
    myClass<T>::eek:perator + ( const myClass<T>& cls ) {
    return *this = cls; // that's probably not what you intended?
    }


    best,

    Michael.
     
    , Apr 10, 2008
    #5
  6. On Apr 10, 12:12 pm, Barry <> wrote:
    > רמי wrote:
    > > Hey,

    >
    > > I've created a small class that overrides the "+" operator, and
    > > defines a destructor:

    >
    > > private:
    > > T *data;
    > > int size;
    > > public:
    > > myClass(int s);
    > > myClass<T> operator+(const myClass<T>& vec);
    > > ~myClass();

    >
    > > The implementation of the operator is:
    > > myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    > > {
    > >                 int i;
    > >    myClass<T> res(cls.size);

    >
    > >    for (i=0;i<cls.size;i++)
    > >    {
    > >          res.data = data;
    > >    }

    >
    > >    return res;
    > > }

    >
    > > Don't judge the code as its just a pseudo of the real one that.

    >
    > > The destructor is:

    >
    > > myClass<T>::~myClass()
    > > {
    > >               delete [] data;
    > > }

    >
    > > The problem is, that after the last line of the operator+ - "return
    > > res"
    > > I don't know why, but the destructor is called for "res",
    > > Which means that the object returned, is already cleaned!!!

    >
    > a temporary instance of myClass<T> is contructed via "Copy Constructor"
    > to copy "res" you just returned. At the end of the function block, "res"
    > is destructed.
    >
    > But RVO/NRVO ([Named] Return Value optimization) technique is used to
    > avoid the copy of for the temp variable.
    >
    > you can google it.- Hide quoted text -
    >
    > - Show quoted text -


    So, how am I suppose to pass an instance of myClass<T>?

    --sternr
     
    רמי, Apr 10, 2008
    #6
  7. On Apr 10, 12:48 pm, wrote:
    > On 10 Apr., 10:41, רמי <> wrote:
    > [..]
    >
    >
    >
    >
    >
    > > The implementation of the operator is:
    > > myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    > > {
    > >                 int i;
    > >         myClass<T> res(cls.size);

    >
    > >         for (i=0;i<cls.size;i++)
    > >         {
    > >               res.data = data;
    > >         }

    >
    > >         return res;

    >
    > > }

    >
    > [..]
    > > The problem is, that after the last line of the operator+ - "return
    > > res"
    > > I don't know why, but the destructor is called for "res",
    > > Which means that the object returned, is already cleaned!!!

    >
    > > What am I missing\doing wrong?

    >
    > Your implementation creates a local variable res that is destroyed
    > when leaving the function body, hence the destructor call.
    > "return res" provides a copy of res to the caller.
    >
    > However, in the member "data", a copy of the pointer is provided that
    > is used to store your array in local variable. When your destructor
    > frees the memory, the pointer in the returned object becomes invalid.
    > You have two options:
    > 1. provide a copy constructor that does a "deep copy". I.e. allocates
    > a new buffer and copies the memory content pointed to.
    > 2. avoid T*, use a std::vector<T> instead. This will do all memory
    > allocation issues transparently for you, no need for memory
    > deallocation in a destructor, no need to write a copy constructor
    > yourself.
    >
    > The solution 2. is what I would prefer. Your adapted class may look
    > like this:
    >
    > template <class T>
    > class myClass {
    > private:
    >     std::vector<T> data;
    >     /* int size; */ // obsolete, use data.size() if you have to.
    >
    > public:
    >     myClass( int s );
    >     myClass<T> operator+( const myClass<T>& vec );
    >     /* ~myClass(); */ // obsolete, default destructor handles data
    > member
    >
    > };
    >
    > If "data" is *the* central member of your class, to make handling more
    > convenient I'd also consider inheritance:
    >
    > template <class T>
    > class myClass : private std::vector<T> {
    >     ...
    >
    > };
    >
    > The implementation of your + operator will become:
    >
    > template <class T>
    > myClass<T>::eek:perator + ( const myClass<T>& cls ) {
    >     return *this = cls; // that's probably not what you intended?
    >
    > }
    >
    > best,
    >
    >    Michael.- Hide quoted text -
    >
    > - Show quoted text -


    Ok, so I understand that if I'll add a:
    myClass(const myClass<T>& cls);

    it will work.

    But what if I overloaded the operator= as well (for other reasons),
    Than if I perform this line:
    cls3 = cls1 + cls2

    What will happen is that 2 instances will be created:
    One from "cls1 + cls2" - by the new copy constructor,
    And another from the "cls3 = result" by the operator=

    How can I minimize that?

    Thanks all

    --sternr
     
    רמי, Apr 10, 2008
    #7
  8. רמי

    Guest

    On 10 Apr., 12:08, רמי <> wrote:
    [..]
    > What will happen is that 2 instances will be created:
    > One from "cls1 + cls2" - by the new copy conad structor,
    > And another from the "cls3 = result" by the operator=
    >
    > How can I minimize that?


    Switch on optimization. Modern compilers will detect the situation and
    generate code that avoids excessive copying of objects on return. I.e.
    they optimize away the local variable completely and perform the
    operation directly in the returned temporary.

    Ancient g++ offered as proprietary extension (a later deprecated and
    removed feature) an explicitly declared return object.

    best,

    Michael
     
    , Apr 10, 2008
    #8
  9. רמי wrote:
    > I've created a small class that overrides the "+" operator, and
    > defines a destructor:
    >
    > private:
    > T *data;
    > int size;
    > public:
    > myClass(int s);
    > myClass<T> operator+(const myClass<T>& vec);
    > ~myClass();
    >
    > The implementation of the operator is:
    > myClass<T> myClass<T>::eek:perator+(const myClass<T>& cls)
    > {
    > int i;
    > myClass<T> res(cls.size);
    >
    > for (i=0;i<cls.size;i++)
    > {
    > res.data = data;
    > }
    >
    > return res;
    > }
    >
    > Don't judge the code as its just a pseudo of the real one that.
    >
    > The destructor is:
    >
    > myClass<T>::~myClass()
    > {
    > delete [] data;
    > }
    >
    > The problem is, that after the last line of the operator+ - "return
    > res"
    > I don't know why, but the destructor is called for "res",
    > Which means that the object returned, is already cleaned!!!
    >
    > What am I missing\doing wrong?


    Since this is a classic violation of the "Rule of three", I wonder why
    nobody else mentioned it:
    http://en.wikipedia.org/wiki/Rule_of_three_(C++_programming)

    You most probably should define a destructor, a copy constructor and a copy
    assignment operator, if you define one of them.

    --
    Thomas
    http://www.netmeister.org/news/learn2quote.html
    sigfault
     
    Thomas J. Gritzan, Apr 10, 2008
    #9
  10. On Apr 10, 1:30 pm, wrote:
    > On 10 Apr., 12:08, רמי <> wrote:
    > [..]
    >
    > > What will happen is that 2 instances will be created:
    > > One from "cls1 + cls2" - by the new copy conad structor,
    > > And another from the "cls3 = result" by the operator=

    >
    > > How can I minimize that?

    >
    > Switch on optimization. Modern compilers will detect the situation and
    > generate code that avoids excessive copying of objects on return. I.e.
    > they optimize away the local variable completely and perform the
    > operation directly in the returned temporary.
    >
    > Ancient g++ offered as proprietary extension (a later deprecated and
    > removed feature) an explicitly declared return object.
    >
    > best,
    >
    >    Michael


    Hey Michael,
    Thanks for the fast reply ;)
    Are you sure there's no "coding" solution for this duality?

    --sternr
     
    רמי, Apr 10, 2008
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. John Smith
    Replies:
    2
    Views:
    434
    Ivan Vecerina
    Oct 6, 2004
  2. Replies:
    4
    Views:
    322
  3. Replies:
    11
    Views:
    750
    James Kanze
    May 16, 2007
  4. Replies:
    11
    Views:
    570
  5. Replies:
    2
    Views:
    323
Loading...

Share This Page