Click here to Skip to main content
15,942,580 members
Please Sign up or sign in to vote.
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

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.
 
Share this answer
 
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
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.
 
Share this answer
 
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
 
Share this answer
 
Comments
malifer 23-Jan-11 17:41pm    
thanks Ash. someone else told me i could do my = operator simply as

stickfigure& operator = (const stickfigure& ref)
{
*test = *(ref.test);
return *this;
}

so no need to test for self assignment or delete. the thing with the design was i wanted to try a sample that wasnt a combination of string and *char but a pointer to some other type. all the assignment operators i study in books the sample is always a char*. I do agree the example i typed up wasnt the best experiment using a standard int. I have a little confusion now in terms of watching out for self assignment now and the issue of loosing a pointer to data so its never released. i thought i had this but now i see that i dont. very frustrating.

also thanks for the ostream demonstration. thats a pretty neat feature i may have to play with that since i seem to use cout quite alot.

well then i guess im stuck on assignment operators then..
Set the pointer to null after delete. Some programmers call the destructor variables go out of scope and some templates do as well.
 
Share this answer
 
Comments
Sandeep Mewara 23-Jan-11 3:24am    
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

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



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900