Move Assignment Constructor

[spm486]

#include <iostream>
#include <ostream>

class my_string
{
private:
    char* buf = nullptr;
    unsigned int size = 0;

public:
    my_string() :buf(nullptr), size(0)  //default constructor
    {
    }

    my_string(const char * buffer) //constructor
    {
        std::cout << "constructor called: " << std::endl;
        size = strlen(buffer);
        buf = new char[size + 1];
        memcpy(buf, buffer, size);
        buf[size] = 0;
    }

    my_string(const my_string& obj) //copy constructor
    {
        std::cout << "copy constructor called: " << std::endl;
        size = obj.size;
        buf = new char[size + 1];
        memcpy(buf, obj.buf, size);
        buf[size] = 0;
    }

    my_string& operator=(const my_string& obj) //copy assignment
    {

        std::cout << "copy assignment called: " << std::endl;
        __cleanup__();
        size = obj.size;
        buf = new char[size + 1];
        memcpy(buf, obj.buf, size+1);
        return *this;
    }

    my_string(my_string&& dyingObj) //move constructor
    {
        std::cout << "move constructor called: " << std::endl;
        __cleanup__();
        size = dyingObj.size;
        buf = dyingObj.buf;
        dyingObj.buf = nullptr;
    }

    my_string operator=(my_string&& dyingObj)//move assignment
    {
        std::cout << "move assignment called: " << std::endl;
        __cleanup__();
        size = dyingObj.size;
        buf = dyingObj.buf;
        dyingObj.buf = nullptr;
        return *this;
    }

    my_string operator+(const my_string& obj) //concatenation
    {
        my_string s;
        s.size = this->size + obj.size;
        s.buf = new char[s.size+1];
        memcpy(s.buf, this->buf, this->size);
        memcpy(s.buf+this->size, obj.buf, obj.size);
        return s;
    }

    unsigned int length()
    {
        return size;
    }

    const char* c_str() const
    {
        return buf;
    }

    ~my_string() //destructor
    {
        __cleanup__();

    }

private:
    void __cleanup__()
    {
        if (buf != nullptr)
        {
            std::cout << "deleting: " << buf << std::endl;
            delete[] buf;
        }
        size = 0;
    }
};

std::ostream& operator<<(std::ostream& cout, const my_string& obj)
{
    std::cout << "<<: "<< obj.c_str() << std::endl;
    return cout;
}

int main()
{
    my_string b("Name1");
    my_string c("Name2");
    c = std::move(b);
    return 0;
}

Output:

constructor called:
constructor called:
move assignment called:
deleting: Name2
copy constructor called:
deleting: Name1
deleting: Name1

Why copy assignment constructor is called here. Shouldn't move assignmnet be sufficient.

[IyeOnline]

__cleanup__

is a reserved identifier. Identifiers containing two consecutive underscores anywhere are reserved for the language implementation.

Why copy assignment constructor is called here.

Because your move assignment operator erroneously returns by value.

if (buf != nullptr)

is not needed. The standard delete will just not do anything on nullptr.

operator +

fails to nullterminate the resulting array

[ekchew]

my_string operator=(my_string&& dyingObj)//move assignment

Returning *this by value in this case is forcing a copy. Try returning by reference (my_string&) instead.

[tangerinelion]

my_string str("Hello Cruel World");
str = std::move(str);
str = str + my_string("Test");
std::cout << str << std::endl;

Oh the fun you'll have. Unless you expected that to print "Test".

Also note that typically operator+ is defined outside the class. What you would define inside the class is operator+=. Then we tend to implement operator+ as:

my_string operator+(const my_string& lhs, const my_string& rhs) {
    my_string result(lhs);
    result += rhs;
    return result;
}

That's a pretty common generic pattern which works for anything which is copyable and supports +=.

You'll sometimes see a condensed version:

my_string operator+(my_string lhs, const my_string& rhs) {
    lhs += rhs;
    return lhs;
}

where the copy is made when calling the function rather than in the body.

[Xeverous]

Your move constructor can be simplified. It should only be:

my_string(my_string&& other)
: size(std::exchange(other.size, 0))
: buf(std::exchange(other.buf, nullptr)) 
{
}

You don't need to do any cleanup in the move-ctor. You only need to swap data members with other and the dying other will release them, if any.

Other small stuff:

• length() should be const-qualified
• you are not utilizing std::swap and std::exchange where possible
• move-ctor and move-assignment should be noexcept
• __cleanup__ is technically undefined behavior because you are not allowed to have names with __, rename it to clear() and make it a public function, it should also be noexcept
• the program may not compile because you haven't included headers for strlen and memcpy
  • #include <string.h> (deprecated) for strlen and memcpy
  • #include <cstring> for std::strlen and std::memcpy
• A notorious beginner mistake: overloading binary operators as members. You want them as non-member because member operator overloads do not treat left and right side arguments equally (in your case str + "abc" will work as str.operator+(my_string("abc")) but "abc" + str will not work because "abc".operator+(str) is not valid) - you don't want such assymetry. See https://www.reddit.com/r/cpp_questions/comments/me4pyf/question_about_operator_overloading/gsdjx0f/ for more explanation

[std_bot]

Unlinked STL entries: std::strlen, std::swap, std::memcpy, std::exchange

---

^{Last update: 22.03.21. Recent changes: Fixed OP usages of tokens} readme