A simple String class and its special member functions

Question

I am learning the behavior of C++'s special member function, using a naive String class as example. (The code is modified from this tutorial) Here is the implementation so-question.cpp:

#include <cstring>
#include <iostream>
#include <vector>

using namespace std;

class String {
private:
  uint32_t size;
  char *buffer;

public:
  String() {
    cout << "Default No-Parameter Constructor" << endl;
    this->size = 0;
    this->buffer = nullptr;
  }

  String(const char *name) {
    cout << "Parameterized Constructor" << endl;
    this->size = std::strlen(name);
    this->buffer = new char[this->size];
    memcpy(this->buffer, name, this->size);
  }

  String(const String &other) {
    cout << "Copy Construtor" << endl;
    this->size = other.size;
    this->buffer = new char[this->size];
    memcpy(this->buffer, other.buffer, this->size);
  }

  String &operator=(const String &other) {
    cout << "Copy Assignment Operator" << endl;
    if (this != &other) {
      // The buffer size of `this` is different from the `other`'s. So we need
      // to delete `this` and reallocate memory for `this` with the size of
      // `other`.
      delete[] this->buffer;

      this->size = other.size;
      this->buffer = new char[this->size];
      memcpy(this->buffer, other.buffer, this->size);
    }
    return *this;
  }

  String(String &&other) {
    cout << "Move Constructor" << endl;
    this->size = other.size;
    /**
     * In the copy constructor, we have to allocate memory.
     * In the move constructor, we don't have to allocate any more memory. We
     * just have to change a couple of variables so it's very efficient.
     */
    this->buffer = other.buffer;

    /**
     * NOTE: Here, the destructor of "other" will deallocate "other.buffer" and
     * we've stolen the buffer so we don't want that to happen. So to deal
     * with this all we have to do is set "other.buffer" to point to
     * nullptr. And it is safe to delete a nullptr.
     */
    other.size = 0;
    other.buffer = nullptr;
  }

  String &operator=(String &&other) {
    cout << "Move Assignment Operator" << endl;
    if (this != &other) {
      this->size = other.size;
      /**
       * Here, the object `buffer` to which `this` points, needs to be freed up,
       * because `this` is going to point to `other`'s memory. If it's not done,
       * then we have a memory leak.
       */
      delete[] this->buffer;

      this->buffer = other.buffer;

      other.size = 0;
      other.buffer = nullptr;
    }
    return *this;
  }
  ~String() {
    cout << "Destructor: ";
    // NOTE: a potential bug is here. If `this` has pointed to a nullptr and
    // size is non-zero, then it causes error:
    // Segmentation fault (core dumped)
    for (uint32_t i = 0; i < this->size; ++i) {
      cout << this->buffer[i];
    }
    cout << '\n';
    delete[] this->buffer;
  }

  friend ostream &operator<<(ostream &out, const String &test);
};

ostream &operator<<(ostream &out, const String &test) {
  out << "String: ";
  for (uint32_t i = 0; i < test.size; ++i) {
    out << test.buffer[i] << ' ';
  }
  return out;
}

String get_String() { return String(); }

The String class can be tested with the following code:

int main() {
  String test1;
  test1 = get_String();
  String test2{"test2"};
  String test3{test2};
  cout << "test1: " << test1 << endl;
  test1 = test3;
  cout << "test2: " << test2 << endl;
  cout << "test3: " << test3 << endl;
  cout << "test1: " << test1 << endl;
  vector<String> vec;
  vec.push_back(String("rvalue"));
  vec.push_back(test1);
  cout << "vec[0] " << vec[0] << endl;
  cout << "vec[1] " << vec[1] << endl;
  cout << '\n';

  cout << "Test move-from object:\n";
  test2 = std::move(test1);
  cout << "test2: " << test2 << endl;
  cout << "test1: " << test1 << endl;
  return 0;
}

And the output message is:

Default No-Parameter Constructor
Default No-Parameter Constructor
Move Assignment Operator
Destructor: 
Parameterized Constructor
Copy Construtor
test1: String: 
Copy Assignment Operator
test2: String: t e s t 2 
test3: String: t e s t 2 
test1: String: t e s t 2 
Parameterized Constructor
Move Constructor
Destructor: 
Copy Construtor
Copy Construtor
Destructor: rvalue
vec[0] String: r v a l u e 
vec[1] String: t e s t 2 

Test move-from object:
Move Assignment Operator
test2: String: t e s t 2 
test1: String: 
Destructor: rvalue
Destructor: test2
Destructor: test2
Destructor: test2

And the code can be run with the command g++ -Wall -std=c++17 -std=gnu++17 so-question.cpp && ./a.out.

The result looks good and makes sense to me.
However, I have one question, is it true that in theory and in practice that when using move constructor and move assignment operator, the move-from object shall be invalid. I am asking this because I am just wondering what if I would like to have two objects points to the same memory location. For example,

String test1{"apple"};
String test2;
test2 = std::move(test1);

test1 becomes invalid after it's moved. But what if I want both test1 and test2 both points to the same string in memory apple. (copy can't be used here because then it will not be the same memory) This can be done by changing the code in move assignment operator. But does it violate some software design pattern and should be avoided or there's a known method to address this behavior? Thank you.

Update: Fix the code to make it as minimum reproducible.

Answer 1

Answer to your question

is it true that in theory and in practice that when using move constructor and move assignment operator, the move-from object shall be invalid.

No, the moved-from object must still be valid, however its contents are allowed to be undefined. So for your class:

String test1{"apple"};
String test2{std::move(test1)};
std::cout << test1;

The printing to std::cout should still work. However, it is allowed to print "a p p l e", "" or "w h a t e v e r". You chose to set size to zero and buffer to nullptr, which is valid.

The reason why it must be valid is that the object test1 is still alive here, and at some point its destructor will be called.

But what if I want both test1 and test2 both points to the same string in memory apple. (copy can't be used here because then it will not be the same memory) This can be done by changing the code in move assignment operator. But does it violate some software design pattern and should be avoided or there's a known method to address this behavior?

Then test1 would not be valid anymore, as both its destructor and that of test2 would try to delete[] the same memory, which is not valid.

A more elegant way to move

The move constructor and move assignment operator become more elegant to write if you implement a swap() member function first, which in turn uses std::swap() on the member variables, and by using default member initializers:

class String {
    std::size_t size = 0;
    char *buffer = nullptr;

public:
    String() {
        std::cout << "Default No-Parameter Constructor\n";
        // Nothing to do anymore!
    }
    
    String(String &&other) {
        std::cout << "Move Constructor\n";
        swap(other);
    }
    
    String &operator=(String &&other) {
        std::cout << "Move Assignment Operator\n";
        swap(other);
        return *this;
    }
    
    void swap(String& other) {
        std::swap(size, other.size);
        std::swap(buffer, other.buffer);
    }
    …
};

You no longer have to check if this == &other, as std::swap() will do the right thing if it swaps the same member variables. The move constructor's body will run after it initialized size and buffer, so other will end up with 0 and nullptr. The move assignment operator will swap the two strings. As mentioned above, that's also valid. The destructor of other will then take care of deleting the old string.

Unnecessary use of this->

You almost never have to write this-> in C++. I would just remove it everywhere, as it just adds noise.

Prefer '\n' over std::endl

Use '\n' instead of std::endl; the latter is equivalent to the former, but also forces the output to be flushed, which is usually not necessary and might impact performance.

Use std::size_t for sizes, counts and indices

The return value of std::strlen() is a std::size_t. Make sure you store your string's size in a variable of that type as well, otherwise you can run into problems if you ever try to store a string that is longer than a std::uint32_t can represent.

It is good practice to always use std::size_t for anything that deals with the size of arrays or objects in memory, as well as counts and indices related to those.

Answer 2

Don't using namespace std;, as that can be harmful.

Instead, write out std::uint32_t, std::cout, std::memcpy etc. in full. Maintainers (future you, perhaps) will thank you for being clear which identifiers you're using, and you'll save yourself from embarrassment when the standard library adds more identifiers.

---

Don't mix diagnostic messages with output.

We should be using std::clog for these messages so they don't end up being processed by the consumer of the program.

---

Use initializers for class data members.

The constructors here allow the members to default-initialise, then assign values. Instead, we should be using the constructor's initializer list to populate these.

---

Use a larger indentation.

2 spaces per level is hard to distinguish. Most style guides recommend at least 4; Linux kernel style uses tabs instead (normally 8 per level).

An advantage of the larger indents is that they alert you to deeply-nested control structures - normally a sign that the code needs refactoring for easier comprehension.

---

And everything mentioned in G. Sliepen's review.