is this assignment operator ok and copy ctor ok

Question

5.00/5 (1 vote)

See more:

I was wondering if this assignment operator and copy constructor looks ok. I am just starting classes. also would it be good practice or pointless to set the pointer to NULL after delete.

#include <iostream>
#include <string>

using std::cout;
using std::endl;
using std::cin;

class stickfigure
{
public:
    stickfigure(int j)
    : test(new int(j))
    {}
    stickfigure(const stickfigure& ref)
    : test(new int(*ref.test))
    {}
    stickfigure& operator = (const stickfigure& ref)
    {
        if (this != &ref)
        {
            delete test;
            test = new int(*ref.test);
        }
        return *this;
    }
    virtual ~stickfigure(){ delete test;}
    void print() const{cout << *test << endl;}
private:
    int* test;
};

int main(int argc, char** argv)
{
    stickfigure stickone(76);
    stickone.print();
    // copy
    stickfigure sticktwo(stickone);
    sticktwo.print();
    // assignment
    stickfigure stickthree = sticktwo;
    stickthree.print();
}

Posted 22-Jan-11 13:03pm

malifer

Add a Solution

4 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Nish Nishant · Accepted Answer · 2011-01-22T13:08:00

Solution 1

Looks good to me. Since you delete test in the destructor, setting it to NULL won't affect anything. People normally do that when the variable can be delete'd/new'd arbitrarily within the lifetime of the object, in that case it's good to be able to check the field for validity and so some people do set it to NULL after delete-ing it.

Posted 22-Jan-11 13:08pm

Nish Nishant

Updated 22-Jan-11 13:09pm

v3

Comments

Sandeep Mewara 23-Jan-11 3:23am

Comment from OP:
ok thanks guys. I just want to make sure im doing the right thing. just reading straight text all the time without any interaction its nice to know im on the right track

Emilio Garavaglia · Accepted Answer · 2011-01-22T20:34:00

An OT emprovement:
instead of stickfigure::print, into stickfigure declare

C++

friend std::ostream& operator<<(std::ostream& s, const stickfigure& z)
{
    return s << *z.test;
}

so that, in main, you can do

cout << stickone << endl << sticktwo << endl << stickthree << endl;

This will work also if cout will be replaced with whatever file stream.

Aescleal · Accepted Answer · 2011-01-23T08:03:00

Your copy constructor looks fine (ignoring the whole "why store an int as a dynamic object" thing) but your assignment operator is not exception safe or neutral. If the new in the assignment operator throws then your object is in an indeterminate state.

If you use the copy and swap idiom you get both exception safety and neutrality AND scarily make the assignment operator three lines long without any logic getting in the way:

stickfigure& operator = (const stickfigure& ref)
{
    stickfigure temp( ref );
    std::swap( ref.test, test );
    return *this;
}

It's an idiom well worth knowing and is pretty well described in "Exception C++" by Herb Sutter. When you get through your introductory text book it's well worth a read.

Cheers,

Ash

T2102 · Accepted Answer · 2011-01-22T13:10:00

Solution 2

Set the pointer to null after delete. Some programmers call the destructor variables go out of scope and some templates do as well.

Posted 22-Jan-11 13:10pm

T2102

Comments

Sandeep Mewara 23-Jan-11 3:24am