Implementing a C++11 raw string class

Question

I'm trying to implement a RawString class that conforms to C++11:

#include <iostream>
#include <string>
#include <memory>

class RawString
{
    private:
        char* buff_;
    public:
        RawString()
        {
            std::cout << "rawstring def ctor\n";
            buff_ = nullptr;
        }
        ~RawString()
        {
            delete [] buff_;
        }
        RawString(const char* str) {
            std::cout << "rawstring normal char* ctor\n";
            buff_ = new char[strlen(str) + 1];
            strcpy(buff_, str);
            buff_[strlen(str)] = '\0';
        }
        RawString(const RawString& str) : RawString(str.buff_) {
            std::cout << "rawstring copy ctor\n";
        }
        RawString(RawString&& other) {
            std::cout << "rawstring move ctor\n";
            buff_ = other.buff_;
            other.buff_ = nullptr;
        }
        RawString(char*&& other) {
            std::cout << "rawstring move ctor\n";
            buff_ = other;
            other = nullptr;
        }
        void swap(RawString& other) {
            std::swap(other.buff_, buff_);
        }
//      RawString& operator=(const char* str) {
//          std::cout << "operator=char*\n";
//          buff_ = new char[strlen(str) + 1];
//          strcpy(buff_, str);
//          buff_[strlen(str)] = '\0';      
//          return *this;
//      }
        RawString& operator=(RawString other) {
            std::cout << "opeartor=\n";
            swap(other);
            return *this;
        }
        friend const RawString operator+(const RawString& lhs, const RawString& rhs) {
            std::cout << "friend opt+\n";
            char* buff = new char[strlen(lhs.buff_) + strlen(rhs.buff_) + 1];
            strcpy(buff, lhs.buff_);
            strcat(buff, rhs.buff_);
            buff[strlen(lhs.buff_) + strlen(rhs.buff_)] = '\0';
            return std::move(buff);
//          RawString newStr(buff);
//          delete[] buff;
//          return newStr;
        }
};

int main()
{
    RawString str1("how are you");
    RawString str2(str1);
    RawString str3 = str1;
    RawString str4;
    str4 = str1;
    str4 = "how are you";
    RawString str5 = str1 + str2;
    return 0;
}

1. Is my implementation correct?
2. Should swap be friend?
3. Should I implement the RawString& operator=(const char* str) assignment operator?
4. Do I need to implement the char* move constructor?
5. Could you give me some additional suggestions?

Answer 1

#include <string>

strcpy and its friends reside within the C library header <string.h>, which should be included as <cstring> into a C++ program. Those functions then should be prepended with std::, e.g. std::strcpy. The C++ library versions defined in the C++ headers have overloads, which fixes some issues.

---

char* buff_;

You can replace that with std::unique_ptr<char[]>. You won't have to implement the move functions nor the destructor when using a unique_ptr data member.

Arguably, you should use a separate class for the whole memory management, following the Single Responsibility Principle. std::vector<char> is a candidate, for example.

---

RawString(const char* str) {
    std::cout << "rawstring normal char* ctor\n";
    buff_ = new char[strlen(str) + 1];
    strcpy(buff_, str);
    buff_[strlen(str)] = '\0';
}

The C library string functions need to compute the length of the strings at each step. This imposes additional, unnecessary work. Additionally, the last step null-terminating buff_ should be unnecessary. Both strlen and strcpy already require that str is null-terminated, and strcpy copies the null-terminator.

RawString(const char* const str) {
    std::cout << "rawstring normal char* ctor\n";

    auto const src_len = std::strlen(str);
    auto const src_len_with_nullterminator = len + 1;

    buff_ = new char[src_len_with_nullterminator];

    std::copy(str, str+src_len_with_nullterminator, buff_);
}

Also note that a user might already know the length of the string passed in; yet, there's no way to tell RawString this length.

---

RawString(const RawString& str)

If you do not have an very memory-constrained system, consider adding a data member that stores the size of the string. You won't have to recompute it in this and other member functions.

---

RawString(RawString&& other)

Your move constructor and separate move assignment-operator (see below) as well as swap should be marked as noexcept. Not only exposes this a useful guarantee to the user via a language feature, but it is also important for other types that make their behaviour dependent on the noexcept-ness of those special member functions. The usual example is std::vector<RawString>, which will provide some more efficient functions (push_back etc.) when the value type has noexcept-move-functions.

---

RawString(char*&& other) {
    std::cout << "rawstring move ctor\n";
    buff_ = other;
    other = nullptr;
}

For convenience, the converting constructor char const* -> RawString (similarly to std::string) is typically implicit, even though it (potentially) is not cheap to construct a RawString.

Your "moving" converting constructor is a constructor that takes ownership of a piece of memory. In my humble opinion, constructors that take ownership should be explicit. For example, it is rather simple to write this bug:

struct filesystem_entry
{
    char name[FILENAME_MAX];
};

auto x = filesystem_entry{};
RawString s = x.name;
// operate on `s` for convenience

Similarly, implicit conversions appear e.g. for returning or passing arguments to a function (where the target type is not immediately visible). The issue is not solved entirely by making the constructor explicit, though:

RawString s( name_of_file() );

If one uses direct-initialization by default, this could be realistic code in my opinion.

Of course, one could argue that such behaviour is defined precisely by the interface of RawString. I'd be surprised by such behaviour, though.

It is much safer to explicitly request a RawString to take ownership of a raw pointer:

RawString(std::unique_ptr<char[]> str) noexcept
    : buff_( str.release() )
{}

---

void swap(RawString& other) {
    std::swap(other.buff_, buff_);
}

Nowadays, the guideline is: Implement swap as a non-member friend function. This allows a generic usage of swap as follows:

using std::swap;
swap(x, y);

This works for both built-in types and user-defined types alike.

Note that since you have an efficient move ctor and move assignment-operator, implementing swap yourself is an optimization.

---

// RawString& operator=(const char* str)

With the OP's current implementation of operator=(RawString), implementing this assignment-operator is a more efficient than the implicit conversion used when it is not declared (const char* -> RawString implicit conversion, then using RawString::operator= (RawString)). However, if you implement an operator=(RawString&&), the current implementation is no more efficient than the implicit conversion used without operator=(const char*).

It is possible though to implement an optimization for this as well as the RawString::operator=(RawString const&) operator (see below), which makes this function useful.

---

RawString& operator=(RawString other)

Although this copy-and-swap technique is simple, it is not the most efficient^(*): If the right hand side of the assignment is an lvalue, and the left hand side already has enough capacity to store the string of the RHS, you waste an allocation:

RawString myString("Hello World");
auto cpy = myString;
myString = cpy; // unnecessary allocation

It is therefore useful to define both separately:

RawString& operator=(RawString const&);
RawString& operator=(RawString&&);

If you follow my advice from above and move the memory management to another class, you can use the compiler-provided implementation for both in RawString.

^(*) Howard Hinnant often refers to this, e.g. in this StackOverflow answer. See also: Herb Sutter on "Don't overuse pass-by-value"

---

friend const RawString operator+(const RawString& lhs, const RawString& rhs)

Although it's almost a convention now, Alexander Stepanov (the inventor of the STL) opposes the usage of + to concatenate strings. He argues that string concatenation is not commutative.

Anyway, in this implementation, we can see a possible inefficiency in the OP's allocation scheme. Classes like std::string and std::vector typically manage a block of memory that is only partially filled. This allows resizing by a certain factor once more memory is needed. An amortized constant complexity can be reached for growth this way.

Also, the OP's implementation uses C library string functions again.

Another inefficiency is that operator+ ignores the value category of its operands. Although an operator like + is supposed to not modify its operands, it can do so if they're rvalues. This allows stealing the resources of any operand. For a similar reason, returning const objects by values is detrimental. Consider:

RawString x("Hello");
auto s = x + " " + "World!";

We get a temporary string from x + " ", but cannot use its resources for the concatenation of + "World!", even if its buffer had enough capacity. Furthermore, the initialization of s requires yet another copy, since the returned RawString is const. s will be a const RawString, which prevents of course modification, even though that could be useful. Because of copy elision, no copy is made for the initialization of s. Similarly, no copy is made for an assignment of the same form, if the operator= takes a RawString by value - on the other hand, declaring operator=(RawString&&) and returning a non-const RawString from operator+ should be no less efficient.

If you provide operator overloads, you should also make sure that there is some consistency. For example, operator+ corresponds to an operator+=.

---

Speaking about operator overloads, an operator== and operator< (as well as its counterparts like >) are generally very useful, e.g. for sorting.

(This leads to regular types.)

---

Oh, and by the way: There's currently no public interface to do something with the contents of the string, sadly. Depending on this interface, the default constructor might be dangerous: It does not initialize buff_ with the address of a null-terminated string (this is a nice class invariant).

---

Late addition: Oops, I missed a critical bug, related to that "nice class invariant" I mentioned above: This class invariant / guarantee is assumed to exist e.g. in the copy constructor:

RawString(const RawString& str) : RawString(str.buff_)

The converting constructor that is the target of the constructor delegation requires the argument to be a null-terminated string (because of the use of strlen and friends).

Similarly, operator+ uses the same assumption.

However, this invariant is violated by the default constructor:

RawString()
{
    std::cout << "rawstring def ctor\n";
    buff_ = nullptr;
}

I can see two relatively simple solutions that maintain the class invariant:

RawString()
{
    std::cout << "rawstring def ctor\n";
    buff_ = new char[1]();
}

This makes the default constructor not noexcept (even though it's unlikely to throw), and it's probably not an efficient solution because of the allocation.

Solution two requires some more work in the destructor:

private:
static char* empty_string() {
    static char str[] = {0};
    return &str;
}

public:
RawString()
{
    std::cout << "rawstring def ctor\n";
    buff_ = empty_string();
}

~RawString()
{
    if(empty_string() != buff_) delete[] buff_;
}

Another possibility is to give up on that class invariant, and allow buffer_ == nullptr. As Loki Astari mentions in a comment to the OP, the bug can be fixed in that case by altering the code in RawString(char const*) and operator+.