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

Implementation of cstring wrapper class

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

Problem

I want to have a thin wrapper (low overhead) around c-style string. Also, with some additional features like == and < operators for map indexing. I have come up with the following class.

Can somebody comment on it? From what I've gathered this class would take only as much memory as a pointer, so much less than std::string.

I also heard that this class has more overhead (execution-wise) than std::string and indeed that is the case for the constructor (char *), copy constructor and move constructor. Can somebody explain why?

inline char * dupStr(const char * s){
    if(!s){return nullptr;}
    size_t l = strlen(s) + 1;
    char * d = new char[l];
    memcpy(d,s,l);
    return d;
}

class CString_{
    private:
        const char *s_;

    public:
        CString_(const char *s):s_(dupStr(s)){}
        CString_(const std::string &s):CString_(s.c_str()){}
        CString_(const CString_ &that):s_(dupStr(that.s_)){}
        CString_(CString_ &&that):s_(that.s_){
            that.s_ = nullptr;
        }

        CString_ & operator= (const CString_ & that) = delete;
        CString_ & operator= (CString_ && that) = delete;

        ~CString_(){ delete[] s_;}

        const char * get() const {return s_;}

        bool operator<(const CString_ &that) const{
            return strcmp(s_,that.s_) < 0;
        }

        bool operator==(const CString_ &that) const{
            return strcmp(s_,that.s_) == 0;
        }
};

Solution

Answers to Questions


I want to have a thin wrapper (low overhead) around c-style string.

I am not sure this class is 'low-overhead' compared to std::string or even a C-String. Because it still has all the overhead of creating dynamic space and copying.

Low-overhead: To me this means the cost of creating the object. If you use this as you measurement then you have not achieved your goal. The cost is about the same as both a std::string and C-String.

Now. It has memory management abilities of std::string but has a smaller footprint. It is the same size as a C-String but the advantage of memory management.

So it has some extra advantages (just not low overhead).


Also, with some additional features like == and < operators for map indexing.

Which is useful; like it.


Can somebody comment on it?

Not a problem.


From what I've gathered this class would take only as much memory as a pointer.

Probably.

But you can always test this by using sizeof() to find out for sure.


so much less than std::string.

It's probably about one third the size of a std::string (you can't make a definitive statement because the standard does not specify how to implement it).

Now a third sounds like a lot. But if a pointer is 4 bytes then a std::string would probably be 12 bytes. Not a great saving. Especially since the std::string has several other advantages (small string optimization: if the string is less than 12 bytes there is no dynamic memory allocation).


I also heard that this class has more overhead (execution-wise) than std::string and indeed that is the case for the constructor (char *), copy constructor and move constructor. Can somebody explain why?

I don't believe that. Your code is pretty optimal in those terms.

Review of Code

Not sure why this is a standalone function? Why not make it a private member of the class.

inline char * dupStr(const char * s){
    if(!s){return nullptr;}           // Sure. Why save the space
                                      //       Do this on multiple lines.
                                      //       The point is to make the code
                                      //       readable.
    size_t l = strlen(s) + 1;
    char * d = new char[l];
    memcpy(d,s,l);
    return d;
}


The move constructor. You should mark the move constructor as noexcept:

CString_(CString_ &&that) noexcept
            : s_(that.s_)
        {
            that.s_ = nullptr;
        }


This is because the standard containers require this to do an optimal operations. If you don't specify this then they will fall back to using the copy constructor (note: only mark it as noexcept if it really is).

Sure you can do a get:

const char * get() const {return s_;}


But it is sometimes also useful to define the cast operator:

const char * operator() const {return get();}


This is useful so you can use CString_ in any location that you could use a C-String without having to change the code.

Low-Overhead.

If I was thinking low-overhead. Then I would make the class take ownership of the string object. That way there is no dynamic allocation or copying. But you get the advantages of RAII (so automatic destruction).

In addition I would get store the size. One of the largest cost of a C-String is repeatedly calculating the size of the string (we switched some old code from using C-String to std::string (a big job)). But we got a 40% speed increase.

The problem is how the C-String was created (malloc() or new). We can make the assumption that the user will only use string that come from C libraries. The problems are that you can't check and make that guarantee (one of the reasons std::string makes a copy).

```
class CStringOwner
{
char* data;
std::size_t size;
public:
CStringOwner(std::nullptr_t)
: data(nullptr)
, size(0)
{}
explicit CStringOwner(char* data)
: data(data)
, size(data ? strlen(data) : 0) // make sure null ptrs are correctly handled.
{}

// Copy.
CStringOwner(CStringOwner const& copy)
: data(reinterpret_cast(malloc(copy.size+1)))
, size(copy.size)
{
if (data == nullptr) {
throw std::bad_alloc("CStringOwner::CStringOwner: Copy: Failed on Malloc");
}
std::copy(copy.data, copy.data+copy.size+1, data);
}
CStringOwner& operator=(CStringOwner copy)
{
copy.swap(*this);
return *this;
}

// Move
CStringOwner(CStringOwner&& move) noexcept
: data(nullptr)
, size(0)
{
move.swap(*this);
}
CStringOwner& operator=(CStringOwner&& move) noexcept
{
move.swap(*this);
return *this;

Code Snippets

inline char * dupStr(const char * s){
    if(!s){return nullptr;}           // Sure. Why save the space
                                      //       Do this on multiple lines.
                                      //       The point is to make the code
                                      //       readable.
    size_t l = strlen(s) + 1;
    char * d = new char[l];
    memcpy(d,s,l);
    return d;
}
CString_(CString_ &&that) noexcept
            : s_(that.s_)
        {
            that.s_ = nullptr;
        }
const char * get() const {return s_;}
const char * operator() const {return get();}
class CStringOwner
 {
      char*        data;
      std::size_t  size;
      public:
          CStringOwner(std::nullptr_t)
              : data(nullptr)
              , size(0)
          {}
          explicit CStringOwner(char* data)
              : data(data)
              , size(data ? strlen(data) : 0) // make sure null ptrs are correctly handled.
          {}

          // Copy.
          CStringOwner(CStringOwner const& copy)
              : data(reinterpret_cast<char*>(malloc(copy.size+1)))
              , size(copy.size)
          {
              if (data == nullptr) {
                  throw std::bad_alloc("CStringOwner::CStringOwner: Copy: Failed on Malloc");
              }
              std::copy(copy.data, copy.data+copy.size+1, data);
          }
          CStringOwner& operator=(CStringOwner copy)
          {
              copy.swap(*this);
              return *this;
          }

          // Move
          CStringOwner(CStringOwner&& move) noexcept
              : data(nullptr)
              , size(0) 
          {
              move.swap(*this);
          }
          CStringOwner& operator=(CStringOwner&& move) noexcept
          {
              move.swap(*this);
              return *this;
          }

          // Destroy
          ~CStringOwner()
          {
              free(data);
          }
          void swap(CStringOwner& other) noexcept
          {
              using std::swap;
              swap(data, other.data);
              swap(size, other.size);
          }

          // Other shit goes here.
  };

Context

StackExchange Code Review Q#113109, answer score: 8

Revisions (0)

No revisions yet.