patterncppModerate
Implementing a C++11 raw string class
Viewed 0 times
rawimplementingstringclass
Problem
I'm trying to implement a
```
#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;
}
};
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.