🔞 Если принимаете сырой указатель, как параметр - не забудьте проверить его на nullptr. Меньше будете голову ломать при будущем дебаге.

🔞 compareData зачем-то принимает параметры по значению. В смысле, понятно зачем: чтобы было double free, а вам было интереснее ревьюить. Лучше все-таки принимать по константной ссылке аргументы.

🔞 Поле buffer публичное как раз для того, чтобы к нему доступ имела compareData. Мы все-таки за инкапсуляцию и мир во всем мире. Поле надо сделать приватным, а compareData другом. А еще лучше убрать compareData и определить нативный оператор сравнения.

Но если очень хочется оставить compareData, то лучше все равно определить оператор, не делать compareData другом класса, и пусть она использует нативный оператор внутри себя без нарушения инкапсуляции.

🔞 Data имеет конструктор от одного аргумента и лучше бы его сделать explicit. В таком случае, чтобы запретить неявные преобразования.

🔞 Многие отметили, что класс стоит пометить как final. Видимо это какой-то кодстайл, чтобы запретить другим классам наследоваться от этого. Причина, как я понял, это отсутствие виртуального деструктора. Поэтому если кто-то хочет отнаследоваться от класса, то он явно видит, что пока классом нельзя пользоваться полиморфно, убирает final и добавляет виртуальный деструктор.

Все же от Data можно безопасно наследоваться, если не использовать потом наследников полиморфно. А при добавлении виртуального метода можно и самому понять, что надо еще и деструктор соотвествующим сделать.

Мне final кажется оверкиллом для одиночных классов в небиблиотечном коде , но тут кто как привык.

Фух, на этом вроде все.

Вот что вышло по итогу исправлений:

class Data final {
private:
    std::unique_ptr<char[]> buffer;
    size_t buf_size = 0;

public:
    Data(const char* input, size_t size)
        : buf_size(size) {
        if (input == nullptr && size != 0) {
            throw std::invalid_argument("Input cannot be null for non-zero size");
        }

        if (size > 0) {
            buffer = std::make_unique<char[]>(size);
            std::copy(input, input + size, buffer.get());
        }
    }

    template <size_t N>
    explicit Data(const char (&str)[N])
        : Data(str, N) {
    }

    // Копирующий конструктор
    Data(const Data& other)
        : Data{other.buffer.get(), other.buf_size} {
    }

    // Универсальное присваивание
    Data& operator=(Data other) {
        swap(*this, other);
        return *this;
    }

    // Перемещающий конструктор
    Data(Data&& other) noexcept {
        swap(*this, other);
    }

    ~Data() = default;

    // Оператор сравнения
    bool operator==(const Data& rhs) const noexcept {
        return std::string_view{buffer.get(), buf_size} == std:;string_view{rhs.buffer.get(), buf_size()};
    }

    // Вспомогательная функция для обмена
    friend void swap(Data& first, Data& second) noexcept {
        using std::swap;
        swap(first.buffer, second.buffer);
        swap(first.buf_size, second.buf_size);
    }
};

[[nodiscard]] bool compareData(const Data& data1, const Data& data2) noexcept {
    return data1 == data2;
}

Пишите, если что забыл.

Critique your solution. Stay cool.

#cppcore #OOP #goodpractice #design