You Can Add private Member Functions and friend Functions

At least, I think you can. That doesn’t change the the public API one whit, or the ABI (so long as none of them are virtual) so it ought to be kosher. I would start with constexpr String::swap(String& other) noexcept and constexpr String::String(char* const moved_data, const std::size_t other_size) noexcept.

The value of the swap is that it lets you use the copy-and-swap idiom, which solves many of your exception-safety bugs. This also makes it much easier to write a lock-free, thread-safe implementation.

The value of a private: constructor that sets data and size is that you frequently default-initialize a String object, which allocates one byte of memory, and then never delete[] its memory. You could insert a call to newstr::~String(); between the declaration and the initialization each and every time, but it would be much better to be able to just write:

String newstr(new_data, new_size);

Use the Correct String-Copy Functions

G. Sleipen already brought up that C++ strings can contain '\0' bytes. They didn’t mention that there are several different places where this causes a bug, such as:

String::String(const String& other) {
    size = other.size;
    data = new char[size + 1];
    std::strcpy(data, other.data);
}

This will fail to copy any data after a \0. Everywhere you use strcpy() in this code, you should be using strncpy(). This also will have the advantage of being faster.

Prefer Member-Initializer Lists in your Constructors

Code such as

constexpr String(char* const moved_data, const std::size new_size) noexcept
 : data(moved_data), size(new_size)
{}

makes it easy to verify that each data member is initialized once and only once. Compilers are very good at optimizing constructors that have this form. It’s also, in my opinion, elegant. (The one gotcha is that the data members will be set in the order that they are declared within your class definition, not the order in which you write the commands.)

In some cases, this form of initialization would force you to write a helper (or “phi”) function, such as

String::String(const String& other)
 : data(new_copy(other.data, other.size)), size(other.size)
{}

If new_copy is a useful function that you will re-use in several places and simplify your implementations, this is a big win.

Use Your Functions to Implement Each Other

The interviewer is testing you to see if you implement operators like = and + by delegating to simpler building blocks, such as the copy constructor.

You Can Add private Member Functions and friend Functions

At least, I think you can. That doesn’t change the the public API one whit, or the ABI (so long as none of them are virtual) so it ought to be kosher. I would start with constexpr String::swap(String& other) noexcept and constexpr String::String(char* const moved_data, const std::size_t other_size) noexcept.

The value of the swap is that it lets you use the copy-and-swap idiom, which solves many of your exception-safety bugs. This also makes it much easier to write a lock-free, thread-safe implementation.

The value of a private: constructor that sets data and size is that you frequently default-initialize a String object, which allocates one byte of memory, and then never delete[] its memory. You could insert a call to newstr::~String(); between the declaration and the initialization each and every time, but it would be much better to be able to just write:

String newstr(new_data, new_size);

Use the Correct String-Copy Functions

G. Sleipen already brought up that C++ strings can contain '\0' bytes. They didn’t mention that there are several different places where this causes a bug, such as:

String::String(const String& other) {
    size = other.size;
    data = new char[size + 1];
    std::strcpy(data, other.data);
}

This will fail to copy any data after a \0. Everywhere you use std::strcpy in this code, you should be using std::memcpy. This also will have the advantage of being faster.

Prefer Member-Initializer Lists in your Constructors

Code such as

constexpr String(char* const moved_data, const std::size new_size) noexcept
 : data(moved_data), size(new_size)
{}

makes it easy to verify that each data member is initialized once and only once. Compilers are very good at optimizing constructors that have this form. It’s also, in my opinion, elegant. (The one gotcha is that the data members will be set in the order that they are declared within your class definition, not the order in which you write the commands.)

In some cases, this form of initialization would force you to write a helper (or “phi”) function, such as

String::String(const String& other)
 : data(new_copy(other.data, other.size)), size(other.size)
{}

If new_copy is a useful function that you will re-use in several places and simplify your implementations, this is a big win.

Use Your Functions to Implement Each Other

The interviewer is testing you to see if you implement operators like = and + by delegating to simpler building blocks, such as the copy constructor.

You Can Add private Member Functions and friend Functions

At least, I think you can. That doesn’t change the the public API one whit, or the ABI (so long as none of them are virtual) so it ought to be kosher. I would start with constexpr String::swap(String& other) noexcept and constexpr String::String(char* const moved_data, const std::size_t other_size) noexcept.

The value of the swap is that it lets you use the copy-and-swap idiom, which solves many of your exception-safety bugs. This also makes it much easier to write a lock-free, thread-safe implementation.

The value of a private: constructor that sets data and size is that you now frequently declare a String object, which allocates one byte of memory, and then never delete[] its memory. You could insert a call to newstr::~String(); between the declaration and the initialization each and every time, but it would be much better to be able to just write:

String newstr(new_data, new_size);

Use the Correct String-Copy Functions

G. Sleipen already brought up that C++ strings can contain '\0' bytes. They didn’t mention that there are several different places where this causes a bug, such as:

String::String(const String& other) {
    size = other.size;
    data = new char[size + 1];
    std::strcpy(data, other.data);
}

This will fail to copy any data after a \0. Everywhere you use strcpy() in this code, you should be using strncpy(). This also will have the advantage of being faster.

Prefer Member-Initializer Lists in your Constructors

Code such as

constexpr String(char* const moved_data, const std::size new_size) noexcept
 : data(moved_data), size(new_size)
{}

makes it easy to verify that each data member is initialized once and only once. It’s also, in my opinion, elegant. (The one gotcha is that the data members will be set in the order that they are declared within your class definition, not the order in which you write the commands.)

In some cases, this form of initialization would force you to write a helper (or “phi”) function, such as

String::String(const String& other)
 : data(new_copy(other.data, other.size)), size(other.size)
{
 }

If new_copy is a useful function that you will also call from other parts of your implementation, this is a big win. Otherwise, you might like what you have now better.

You Can Add private Member Functions and friend Functions

At least, I think you can. That doesn’t change the the public API one whit, or the ABI (so long as none of them are virtual) so it ought to be kosher. I would start with constexpr String::swap(String& other) noexcept and constexpr String::String(char* const moved_data, const std::size_t other_size) noexcept.

The value of the swap is that it lets you use the copy-and-swap idiom, which solves many of your exception-safety bugs. This also makes it much easier to write a lock-free, thread-safe implementation.

The value of a private: constructor that sets data and size is that you frequently default-initialize a String object, which allocates one byte of memory, and then never delete[] its memory. You could insert a call to newstr::~String(); between the declaration and the initialization each and every time, but it would be much better to be able to just write:

String newstr(new_data, new_size);

Use the Correct String-Copy Functions

G. Sleipen already brought up that C++ strings can contain '\0' bytes. They didn’t mention that there are several different places where this causes a bug, such as:

String::String(const String& other) {
    size = other.size;
    data = new char[size + 1];
    std::strcpy(data, other.data);
}

This will fail to copy any data after a \0. Everywhere you use strcpy() in this code, you should be using strncpy(). This also will have the advantage of being faster.

Prefer Member-Initializer Lists in your Constructors

Code such as

constexpr String(char* const moved_data, const std::size new_size) noexcept
 : data(moved_data), size(new_size)
{}

makes it easy to verify that each data member is initialized once and only once. Compilers are very good at optimizing constructors that have this form. It’s also, in my opinion, elegant. (The one gotcha is that the data members will be set in the order that they are declared within your class definition, not the order in which you write the commands.)

In some cases, this form of initialization would force you to write a helper (or “phi”) function, such as

String::String(const String& other)
 : data(new_copy(other.data, other.size)), size(other.size)
{}

If new_copy is a useful function that you will re-use in several places and simplify your implementations, this is a big win.

Use Your Functions to Implement Each Other

The interviewer is testing you to see if you implement operators like = and + by delegating to simpler building blocks, such as the copy constructor.