Skip Sailors wrote in message <34f60c53.95126354@news.wyoming.com>...
>Copy c'tors and op=() are different.

Thanks, Skip. And thanks to Valentin as well.

The fact about Copy ctors possibly not quite formed is the best
explanantion. I had yet to do any that allocated resources in the
constructor. And checking for *thi == that is a really good idea, I had
assumed no clients of my class would attempt a = a;

Ed
---
[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

Ed Howland wrote:
>
> [Checking] for *thi == that is a really good idea, I had
> assumed no clients of my class would attempt a = a;

Well, it's something you won't often do directly, but you might do it
indirectly. Slightly contrived example:

    // Library code:
    pair<coord,coord> point;
    void foo(point & p, coord const & x, coord const & y)
    {
        p.first = x;
        p.second = transform_coord(y);
    }

    // User code:
    foo(p, p.first, q.second);

This behaves badly if coord's assignment operator doesn't check. In this
case, foo() should probably use

    p = make_pair(x, transform_coord(y));

but this sort of problem can happen in less obvious ways. It isn't that
unusual to have a function that modifies a whole structure, but you
really only want to change part of it. With a correct operator=(), you
can rely on x=x being a "no-op" to leave the right parts unchanged.
--
Bradd W. Szonye
bradds@concentric.net
http://www.concentric.net/~Bradds


[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

In article: <34EA6B98.2C08@thegrid.net>  Ed Howland <ehowland@thegrid.net> writes:
>
> After several iterations of writing separate copy constructors and
> assignment operators, I noticed that they were almost always the same. I
> reasoned that writing it once in the assignment op and calling that from
> the copy constructor would work:
>
> MyClass::MyClass(const MyClass& that)
> {
>    *this = that;
> }
>
> MyClass& MyClass::operator=(const MyClass& that)
> {
>    m_nMember1 = that.m_nMember1;
>    m_dwMember2 = that.m_dwMember2;
>
>    return *this;
> }
>
> Someone (a respected C++ prof at a local University) once told me that
> this was unsafe. But I forgot the reason why. Does anyone know why?

1/
Because one of the members may be managing a resource...

class Xample
{
public:
    Xample() : p(new Resource) {}
    ~Xample() { delete p; }

    Xample& operator=(const Xample x)
    {
        // Note, no pointless, inefficient, test for "this == &x"
        Resource* tmp(x.p->makeClone());

        // Allocation OK, now safe to update state
        std::swap(tmp, p);

        delete tmp;
    }

#ifdef BUGGY
    Xample(const Xample& x)
    {
        *this = x;  // But "p" contains garbage
    }
#else //Not buggy
    Xample(const Xample& x) :
    p(x.p->makeClone())
    {
    }
#endif

private:
    Resource* p;
};


2/
Because initialising and then assigning a member is less elegant and may (in
some cases) be much more expensive than initialising it right in the first place.


PS

IRL use "std::auto_ptr<Resource>" not "Resource*" - it simplifies the
code.
__
Alan Griffiths              | Senior Systems Consultant, Experian (Formerly CCN)
alan.griffiths@experian.com (agriffiths@ma.ccngroup.com, Tel. +44 115 934 4517)
(ACCU C++ SIG organiser     | <http://www.accu.org/> alan@octopull.demon.co.uk)



[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

Copy c'tors and op=() are different.

Copy c'tors cannot assume that they have well-formed instances from
which to work.  They can assume that there are no resources allocated
to them yet.

op=(), OTOH, _can_ assume that *this is well-formed and must also
assume that resources allocated in a well-formed instance are
allocated to *this as well.

op=() is responsible for releasing resources that may be allocated to
*this before copying that.  It is also a good idea to make sure that
this != that or you risk losing the resource altogether.  Things
dangle.

op=() and the copy c'tor will often share the task of copying members
from one instance to another.  This operation can easily be factored
into a private method that both methods can use.

On 18 Feb 98 13:01:58 GMT, Ed Howland <ehowland@thegrid.net> wrote:

>After several iterations of writing separate copy constructors and
>assignment operators, I noticed that they were almost always the same. I
>reasoned that writing it once in the assignment op and calling that from
>the copy constructor would work:
>...


[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

After several iterations of writing separate copy constructors and
assignment operators, I noticed that they were almost always the same. I
reasoned that writing it once in the assignment op and calling that from
the copy constructor would work:

MyClass::MyClass(const MyClass& that)
{
   *this = that;
}

MyClass& MyClass::operator=(const MyClass& that)
{
   m_nMember1 = that.m_nMember1;
   m_dwMember2 = that.m_dwMember2;

   return *this;
}

Someone (a respected C++ prof at a local University) once told me that
this was unsafe. But I forgot the reason why. Does anyone know why?

Ed


--
Ed Howland MCPS, ACM Member
ehowland@thegrid.net
Proud to part of the 21% who still disapprove
"Never bet on evil", J.C.
---
[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

Ed Howland <ehowland@thegrid.net> wrote:

>After several iterations of writing separate copy constructors and
>assignment operators, I noticed that they were almost always the same. I
>reasoned that writing it once in the assignment op and calling that from
>the copy constructor would work:
>
>MyClass::MyClass(const MyClass& that)
>{
>   *this = that;
>}
>
>MyClass& MyClass::operator=(const MyClass& that)
>{
>   m_nMember1 = that.m_nMember1;
>   m_dwMember2 = that.m_dwMember2;
>
>   return *this;
>}
>
>Someone (a respected C++ prof at a local University) once told me that
>this was unsafe. But I forgot the reason why. Does anyone know why?
>
>Ed

All constructors need to ensure that the object is initialized into a stable
state. The assignment operator can (and must) assume the object is already
initialized. While the compiler will call the default constructor for a members
of a class automatically if not done explicitly, the default construction of
built in type, especially pointers, may not leave the class set up correctly.

Take the example of a smart pointer, the assignment operator may need to do some
processing on the old object before reseating itself. If the copy constructor
did not initialize the pointer this would be a disaster as it is pointing to
some unknown location.

There are also efficiency considerations that the assignment operator may have
some additional work to do at clean up the old values before replacing.

So if you initialize to object before invoking the assignment it should be safe,
but maybe inefficient. The reverse of building the assignment on the copy
constructor is definitely unsafe due to object slicing. What I more often do is
make assignment and copy construction use a shared function to copy the value,
and assignment and the destructors share a cleanup function, and maybe another
common function for constructors to share any common initalization/registration
of the object.

--
richard_damon@iname.com (Redirector to my current best Mailbox)
rdamon@beltronicsInspection.com (Work Adddress)
Richad_Damon@msn.com (Just for Fun)


[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

Ed Howland wrote:

> After several iterations of writing separate copy constructors and
> assignment operators, I noticed that they were almost always the same. I
> reasoned that writing it once in the assignment op and calling that from
> the copy constructor would work:
>
> MyClass::MyClass(const MyClass& that)
> {
>    *this = that;
> }
>
> MyClass& MyClass::operator=(const MyClass& that)
> {
>    m_nMember1 = that.m_nMember1;
>    m_dwMember2 = that.m_dwMember2;
>
>    return *this;
> }
>
> Someone (a respected C++ prof at a local University) once told me that
> this was unsafe. But I forgot the reason why. Does anyone know why?

It seems like it ought to work fine. However, your assignment operator
generally must do two things in addition to what your copy constructor does:

    1. Destroy the lhs object's current contents.
    2. Handle assignment to self appropriately.

These tasks complement each other:

    class MyClass
    {
        int  x;
        char *pName;
        ...
    };

    MyClass& MyClass::operator = (const MyClass &rhs)
    {
    // Must avoid assignment to self because of the delete [],
    // below; if we performed assignment to self as an ordinary
    // assignment, we'd dereference a deleted pointer, which is
    // illegal.

        if (this != &rhs)
        {
        // Just assign simple non-pointer members.

            x = rhs .x;

        // Handle pointer members according to your object ownership
        // plan. For this example, each MyClass object keeps its
        // own copy of whatever pName points to.

            delete [] pName;
            pName = new char [strlen (rhs .pName) + 1];
            strcpy (pName, rhs .pName);
        }
        return *this;
    }

    MyClass::MyClass (const MyClass &rhs)
    {
    // Must assign NULL to all member pointers, because the assignment
    // operator deletes them unconditionally. Deleting a NULL pointer
    // is fine, but deleting an uninitialized pointer is bad news.

        pName = NULL;
        *this = rhs;
    }

---
Good luck,

Ron



[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

Ed Howland <ehowland@thegrid.net> wrote in article
<34EA6B98.2C08@thegrid.net>...
> After several iterations of writing separate copy constructors and
> assignment operators, I noticed that they were almost always the same. I
> reasoned that writing it once in the assignment op and calling that from
> the copy constructor would work:
>
> MyClass::MyClass(const MyClass& that)
> {
>    *this = that;
> }
>
> MyClass& MyClass::operator=(const MyClass& that)
> {
>    m_nMember1 = that.m_nMember1;
>    m_dwMember2 = that.m_dwMember2;
>
>    return *this;
> }
>
> Someone (a respected C++ prof at a local University) once told me that
> this was unsafe. But I forgot the reason why. Does anyone know why?
>
The assignment operator assumes that the object you are assigning to
already exists. Suppose you have following class:
class X {
private:
    char* p;
    // invariant: P always points to a valid string
    X (X&x) { *this = x;}
    // invariant violated in operator=!

    X& operator= (X& x) { char first = p[0]; ... } // problem!
};

Furthermore, the assignment operator can be virtual, which means that you
would
not even be allowed to call it from a copy constructor at all.



[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

[Followups set to comp.lang.c++.moderated; it's a 'how-to'
question, not a language question]

Ed Howland wrote:
>
> After several iterations of writing separate copy constructors and
> assignment operators, I noticed that they were almost always the same. I
> reasoned that writing it once in the assignment op and calling that from
> the copy constructor would work:
>
> MyClass::MyClass(const MyClass& that)
> {
>    *this = that;
> }
>
> MyClass& MyClass::operator=(const MyClass& that)
> {
>    m_nMember1 = that.m_nMember1;
>    m_dwMember2 = that.m_dwMember2;
>
>    return *this;
> }

If operator= and the copy ctor just copy all members, then don't
worry: the compiler will do that for you. Don't declare nor define
them and you get teh default ones.

Else you should implement operator= in term of the copy ctor;
it's simple, safe, nice, even exception safe and excellent style:

MyClass& MyClass::operator= (const MyClass& rhs)
{
    MyClass tmp (rhs); // construct a copy of rhs
                       // using copy ctor
    swap (tmp); // swap *this and tmp, the copy of rhs
    return *this; // tmp (which new contains
                  // the previous *this) is destroyed
}

where swap just swaps all members (and just does that,
never more):

void   MyClass::swap (MyClass& rhs)
{
    swap (m_dwMember1, rhs.m_dwMember1); // for a built-in type
    m_dwMember2.swap (rhs.m_dwMember2); // for annother class
                                        // written that way
}

--

Valentin Bonnard                mailto:bonnardv@pratique.fr
info about C++/a propos du C++: http://www.pratique.fr/~bonnardv/
---
[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]

Ed Howland <ehowland@thegrid.net> wrote:
: After several iterations of writing separate copy constructors and
: assignment operators, I noticed that they were almost always the same. I
: reasoned that writing it once in the assignment op and calling that from
: the copy constructor would work:

: MyClass::MyClass(const MyClass& that)
: {
:    *this = that;
: }

: MyClass& MyClass::operator=(const MyClass& that)
: {
:    m_nMember1 = that.m_nMember1;
:    m_dwMember2 = that.m_dwMember2;

:    return *this;
: }

: Someone (a respected C++ prof at a local University) once told me that
: this was unsafe. But I forgot the reason why. Does anyone know why?

Because it's a logical error. Construction is not an assignement,
assignement is not a construction. Their semantics is totally
different.  You can't implement one in terms of the other. Just forget
about it. At best you can put their common part in a function.

1. It won't work if m_nMember1 or m_dwMember2 is const.

2. It won't work if m_nMember1 or m_dwMember2 doesn't have
   a default constructor.

3. It won't work if for m_nMember1 or m_dwMember2 default constructor
   followed by an assignement has different semantics then copy
   construction.

4. It won't work if the assignement for m_dwMember2 throws.

Instead of thinking of all that, you might just make it a rule to
make your design logically correct first, and physically correct
second. This alone will go a long way in ensuring safety, even
if you don't see all the potential problems with your design.

Oleg.
--
Life is a sexually transmitted, 100% lethal disease.
---
[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@ncar.ucar.edu    ]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://reality.sgi.com/austern_mti/std-c++/faq.html              ]