HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Implementing a C++11 raw string class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rawimplementingstringclass

Problem

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

```
#include
#include
#include

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;
}
};

Solution

#include 


strcpy and its friends reside within the C library header `, which should be included as 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. 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 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, 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 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, a

Code Snippets

#include <string>
char* 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 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_);
}
RawString(const RawString& str)

Context

StackExchange Code Review Q#75790, answer score: 17

Revisions (0)

No revisions yet.