Question

I think the following code is better than copy-and-swap idiom.

In this way, you can use two macros to encapsulate the definitions of copy and move assignment operator. In other words, you can avoid to explicitly define them in your code. As a consequence, you can focus your attention on ctors and dtor only.

Is there any disadvantage of the method?

class A
{
public:
    A() noexcept
        : _buf(new char[128])
    {}

    ~A() noexcept
    {
        if (_buf)
        {
            delete[] _buf;
            _buf = nullptr;
        }
    }

    A(const A& other) noexcept
        : A()
    {
        for (int i = 0; i < 128; ++i)
        {
            _buf[i] = other._buf[i];
        }
    }

    A(A&& other) noexcept
        : _buf(other._buf)
    {
        _buf = nullptr;
    }

    A& operator =(const A& other) noexcept
    {
        if (this != &other)
        {
            this->~A();
            new(this) A(other);
        }

        return *this;
    }

    A& operator =(A&& other) noexcept
    {
        if (this != &other)
        {
            this->~A();
            new(this) A(static_cast<A&&>(other));
        }

        return *this;
    }

private:
    char* _buf;
};
Was it helpful?

Solution

class A
{
public:
    A() noexcept
        : _buf(new char[128])
    {}

In the above, A() will call std::terminate() if new char[128] throws an exception.

    ~A() noexcept
    {
        if (_buf)
        {
            delete[] _buf;
            _buf = nullptr;
        }
    }

In the above, looks ok. Could be simplified down to:

    ~A() noexcept
    {
        delete[] _buf;
    }



    A(const A& other) noexcept
        : A()
    {
        for (int i = 0; i < 128; ++i)
        {
            _buf[i] = other._buf[i];
        }
    }

In the above, will call std::terminate() if new char[128] throws an exception. But otherwise fine.

    A(A&& other) noexcept
        : _buf(other._buf)
    {
        _buf = nullptr;
    }

In the above, looks good.

    A& operator =(const A& other) noexcept
    {
        if (this != &other)
        {
            this->~A();
            new(this) A(other);
        }

        return *this;
    }

In the above, normally I'd say this is dangerous. What if new(this) A(other); throws? In this case, it won't, because if it tries to, the program will terminate. Whether that is safe behavior or not depends on the application (terminate didn't work well for Ariane 5, but works fine in more mundane applications).

    A& operator =(A&& other) noexcept
    {
        if (this != &other)
        {
            this->~A();
            new(this) A(static_cast<A&&>(other));
        }

        return *this;
    }

The above should work fine. Though I'm not sure it is superior to this non-branching version below with otherwise equivalent performance. A behavioral difference is that the version below is not a no-op for self-move-assignment. However it is my belief that self-move-assignment need not be a no-op as one of the post conditions state that the resulting value is unspecified (the other post-condition states that it is specified, leading to a undependable contradiction).

    A& operator =(A&& other) noexcept
    {
        delete [] _buf;
        _buf = nullptr;
        _buf = other._buf;
        other._buf = nullptr;
        return *this;
    }

OTHER TIPS

It will work correctly in the context you provided.

This technique will be disastrous when A is a polymorphic class and has virtual destructor.

You could greatly simplify this class by using unique_ptr<char[]> for _buf:

class A
{
public:
    static const std::size_t bufsize = 128;
    
    A() noexcept
        : _buf(new char[bufsize])
    {}

    A(const A& other) noexcept
        : A()
    {
        copy_from(other);
    }

    A(A&& other) noexcept = default;

    A& operator =(const A& other) noexcept
    {
        copy_from(other);
        return *this;
    }

    A& operator =(A&& other) noexcept = default;

private:
    void copy_from(const A& other) noexcept {
        std::copy_n(other._buf.get(), bufsize, _buf.get());
    }

    std::unique_ptr<char[]> _buf;
};

The class is shorter, more idiomatic, and safer in the face of future changes since it avoids the "clever" delete + placement new. I personally would remove the noexcept from A() and A(const A&), but if you want the program to terminate on allocation failure that's your choice ;)

If your goal is simply to avoid writing assignment operators - and I don't blame you, they're annoyingly banal - you should design to the Rule of Zero:

class A
{
public:
    static const std::size_t bufsize = 128;
    
    A() : _buf(bufsize) {}

private:
    std::vector<char> _buf;
};

There - all implicit copies and moves.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top