1

Following the examples in the online tutorials about the rules of 5, I wrote this class:

#include <iostream>
#include <cstring>
#include <utility>
class A2
{
    char* _buff;
public:
    A2(const char *text = "test") {
        std::cout << "New A2\n";
        _buff = new char[256];
        size_t length = strnlen(text, 255) + 1;
        strncpy(_buff, text, length);
    }
    
    virtual ~A2() {
        std::cout << "Delete A2\n";
        delete[] _buff;  // deallocate
    }
    
    A2(const A2& other) : A2(other._buff) {
        std::cout << "Copy constructor A2\n";
    }
    
    A2(A2&& other) noexcept : _buff(std::exchange(other._buff, nullptr)) {
        std::cout << "Move constructor A2\n";
    }
    
    A2& operator=(const A2& other) {
        std::cout << "Copy assignment A2\n";
        return *this = A2(other);
    }
    
    A2& operator=(A2&& other) noexcept {
        std::cout << "Move assignment A2\n";
        std::swap(_buff, other._buff);
        return *this;
    }
};

And doing some tests it actually seems to work for me (it is copied with some customizations from the examples).

So I tried to create a class that inherits from A2 (in this case, the text as a constructor parameter is also passed to the parent but in their internal management they remain separate):

class B2 final : public A2
{
    char* _buff2;
public:
    B2(const char* text) : A2(text) {
        _buff2 = new char[256];
        size_t length = strnlen(text, 255) + 1;
        strncpy(_buff2, text, length);
        std::cout << "new B2\n";
    }

    ~B2() {
        std::cout << "delete B2\n";
        delete[] _buff2;  // deallocate
    }
    
    B2(const B2& other) : A2(other) {
        _buff2 = new char[256];
        size_t length = strnlen(other._buff2, 255) + 1;
        strncpy(_buff2, other._buff2, length);
        std::cout << "copy constructor B2\n";
    }
    
    B2(B2&& other) noexcept  : A2(static_cast<A2&&>(other)), _buff2(std::exchange(other._buff2, nullptr))
    {
        std::cout << "move constructor B2\n";
    }
    
    B2& operator=(const B2& other) {
        std::cout << "operator = in B2\n";
        A2::operator= (other);
        size_t length = strnlen(other._buff2, 255) + 1;
        strncpy(_buff2, other._buff2, length);
        return *this;
    }
    
    B2& operator=(B2&& other) noexcept {
        std::cout << "move assignment B2\n";
        A2::operator=(static_cast<A2&&>(other));
        std::swap(_buff2, other._buff2);
        return *this;
    }
};

And it seems work fine (I did some testing here too, and checked that everything worked fine with the debugger).

But my doubt is: is what written for the B2 class is correct? I am not very convinced by the direct use of the operators (A2::operator = (other) and A2::operator = (static_cast <A2&&> (other))) to passing parameter to the parent.

Is there any way to write the same code more clearly and correctly?

Zoe
  • 27,060
  • 21
  • 118
  • 148
Andrea
  • 55
  • 7
  • `class B2 { A2 a1; A2 a2; public: B2(const char* text) : a1(text), a2(text) {}}`? – Jarod42 Sep 06 '21 at 09:53
  • 1
    `A2(static_cast(other))` should preferably be written as `A2(std::move(other))` - also, why not allocate just as much memory that's needed? `A2(const char* text, size_t len) : len_(len), buff_(new char[len_ + 1]) { std::copy_n(text, len_ + 1, _buff); }` and `A2(const char* text = "test") : A2(text, std::strlen(text)) {}` – Ted Lyngmo Sep 06 '21 at 09:55
  • Your copy constructor in `A2` does a shallow copy. `A2 a; A2 b = a;` will result in double `delete`. – Yksisarvinen Sep 06 '21 at 10:41
  • 1
    `Is there any way to write the same code more clearly and correctly?` the use of `std::vector` instead of `char *` would make many things easier. And as you use `strncpy` it indicates that it is a string, so using `std::string` might be the better choice? (Sure that does not answer how to do the rule of five correctly in case you want to learn this, but generally, you want to move the error-prone work to the compiler and the std) – t.niese Sep 06 '21 at 10:54
  • 1
    If possible, use **rule of 0** by using only types that can be copied or moved for your members like `std::string` for classes you want to copy or move. Simpler code is easier to understand and less error prone. – Phil1970 Sep 06 '21 at 16:04
  • 2
    Polymorphic hierarchies need copying and moving *disabled*. You never want to pass them by value or assign them, because of object slicing. – n. m. could be an AI Sep 06 '21 at 18:20
  • 1
    Thanks @TedLyngmo, I inserted il `move` and it works fine. About how I used `char*` you understood correctly: this example it's only to learn :) – Andrea Sep 07 '21 at 07:28
  • @Yksisarvinen, but it's correct double delete, because `A2 b=a;` is a copy not a move. – Andrea Sep 07 '21 at 07:29
  • Hi @Phil1970, you are right, but I had entered that code just to complicate things a bit. – Andrea Sep 07 '21 at 07:31
  • @n.1.8e9-where's-my-sharem. thanks, I didn't know about this rule. Now I will try to understand this thing. – Andrea Sep 07 '21 at 07:38
  • 2
    @Andrea It is not an absolute requirement, just a rule of thumb. – n. m. could be an AI Sep 07 '21 at 08:07
  • And if you want copying in such case, you would generally want a `virtual clone` method that return a copy of the same type. Something like `auto b = a->clone();`. – Phil1970 Sep 07 '21 at 23:14

0 Answers0