[fdo-internals] Incorrect implementation of noncopyable concept

[Mateusz Loskot]

Hi,

Recently, I've been searching through sources of the ArcSDE provider
(http://svn.osgeo.org/svn/fdoarcsde/) and I found there is used
concept of noncopyable types.
I'd like to report a bug here - as we don't have bug tracker up and
running yet (or I don't know where it is) - because noncopyability
is misused here
In the provider, all is based on declaring public assignment operator,
but not implementing it intentionally. For example here:

http://svn.osgeo.org/svn/fdoarcsde/trunk/Providers/ArcSDE/Src/Provider/ArcSDEInsertCommand.h

This approach is insufficient to make a type noncopyable, because I can
still call:

A a;
A b(a); // assignment operator is not used

but current solution prevents only this case:

A a
B b;
b = a; // assignment operator is used, but linker will complain

1st issue, is that there is missing copy constructor.
Copy constructor makes (mostly) inseparable pair together with
assignment operator, so they are need to be used together.

Let's assume there is a copy constructor added.

2nd issue, is that the noncopyable concept is based on declaring copy
ctor and assignment op as private, to cause compiler errors but not
linker errors. Compiler errors are meaningful here ie. saying "can not
access private copy ctor" but linker error is mostly meaningless.

Full version of noncopyable concept is implemented as follows:

class A
{
public:
    A() {}
private:
    // Blocked copyability - DO NOT IMPLEMENT
    A(A const&);
    A& operator=(A const&);
};

int main()
{
    A a;
    A b(a); // can not access private A::A()
    A c;
    c = b;  // can not access priavate A::operator=()

    return 0;
}


Obviously, it's easy to forgot about putting private prototypes for
every noncopyable class, so IMHO it's better to use noncopyable
concept already available from Boost:

#include <boost/utility.hpp>

class A : boost::noncopyable
{
public:
    A() {}
};

IMO, it's better because shorter and doesn't bloat a class definition.
Also, note, the inheritance is intentionally *private*, so the relation
between A and noncopyable types is HAS-A (composition)
but not IS-A (inheritance).

Cheers
-- 
Mateusz Loskot
http://mateusz.loskot.net