1
\$\begingroup\$

I have project where I will need to create lots of immutable strings. If I am using std::string, which has huge overhead - about 60-70% against const char *. On a 64-bit machine, the current implementation uses 8 bytes for the class + the const char * size. This is the same size as it would be with plain C.

I will also do optimization with union and will put small strings into the pointer itself.

I need to know if I am on right track.

#include <stdio.h>

#include <memory>
#include <string.h>

class String{
public:
    String(){};

    String(const char *s) : _data(__dup(s)){};

    String(const String & other) : String( other.c_str() ){};

    String(String && other) = default;

    String & operator=(String other){
        std::swap(_data, other._data);
        return *this;
    }



    const char *c_str() const{
        return _data.get();
    }

    int cmp(const String & other) const{
        if (c_str() == nullptr)
            return other ? -1 : 0;

        return strcmp(*this, other);
    }

    operator bool() const{
        return c_str();
    }

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



    bool operator == (const String & other) const{
        return cmp(other) == 0;
    }

    bool operator != (const String & other) const{
        return cmp(other) != 0;
    }

    bool operator > (const String & other) const{
        return cmp(other) > 0;
    }

    bool operator >= (const String & other) const{
        return cmp(other) >= 0;
    }

    bool operator < (const String & other) const{
        return cmp(other) < 0;
    }

    bool operator <= (const String & other) const{
        return cmp(other) <= 0;
    }

private:
    static char *__dup(const char *s){
        auto size = strlen(s);
        char *copy = new char[size];
        memcpy(copy, s, size);
        return copy;
    }

private:
    std::unique_ptr<char[]> _data;
};



void p(String &s){
    printf("%s\n", s ? (const char *) s : "[null]");
}

int main(){
    String s1;          p(s1);
    String s2  = { "hello" };   p(s2);
    String s3a = s2;        p(s3a);
    String s3  = std::move(s3a);    p(s3);
    s1 = s3;            p(s1);
    s1 = "hi";          p(s1);

    String a = "aaaa";
    String b = "bbbb";
    String c = "aaaa";

    printf("%s %s\n", a == c ? "Y" : "N", "Y");
    printf("%s %s\n", a == b ? "Y" : "N", "N");
    printf("%s %s\n", a != b ? "Y" : "N", "Y");
    printf("%s %s\n", a <  b ? "Y" : "N", "Y");
    printf("%s %s\n", a >  b ? "Y" : "N", "N");
    printf("%s %s\n", a <= b ? "Y" : "N", "Y");
    printf("%s %s\n", a >= b ? "Y" : "N", "N");
    printf("%s %s\n", a <= c ? "Y" : "N", "Y");
    printf("%s %s\n", a >= c ? "Y" : "N", "Y");

    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ You probably want __dup to return something, right? Also, I generally wouldn't recommend using printf \$\endgroup\$ – Dannnno Jul 23 '15 at 17:40
  • \$\begingroup\$ __dup() - is a mistake. found it, but forgot to re-post. \$\endgroup\$ – Nick Jul 23 '15 at 18:33
3
\$\begingroup\$

Design

In modern standard libraries, std::string uses short string optimization, which allows to avoid heap allocations at all for string shorter then ~20 chars (on 64-bit machines). So you really want to measure performance, I am not readily convinced that your string offers performance improvement. Of course, for your specific use pattern it might, but you have to measure.

__dup function

This is the most problematic part of the code.

  • Apparently, you forgot to return value from __dup method (now fixed).
  • You must allocate one char more and fill it with zero. Or copy size+1 characters from the source.
  • Identifiers with leading double underscores or one leading underscore and a capital letter are reserved by the standard. You must not use such an identifier. It is enough that you already have put it into private section.

After the corrections, the function could look like this:

static char *dup(const char *s){
    auto size = strlen(s);
    char *copy = new char[size + 1];
    memcpy(copy, s, size + 1);
    return copy;
}

Safety

Functions do not check for null pointers. This code will cause illegal memory access:

String x;
String y{x};

Style

  • You don't need semicolons after the member function definitions
  • I would recommend not putting methods on a single line even if they have empty bodies.
  • You can use C++ style headers <cstring> and <cstdio> instead of C-style ones.
  • I would recommend to implement an operator<<(std::ostream &, const String &) so you could use your string together with iostreams.
  • For testing, it is better to use a testing framework. Like boost.test, gtest, Catch etc.
\$\endgroup\$
  • \$\begingroup\$ thanks a lot! I noted the size and return error, but did not know this about identifiers. This explains why many people put underscore at the end, e.g. dup_(). I know GNU std::string must have short string optimization, but when I did tested it, it never worked for me. Even standard malloc() can not get less than 24 bytes, std:string with "hello" (len=6), give me triple time an 40% more memory consumption than malloc() / memcpy(). I am really surprised Boost did not include string like mine. \$\endgroup\$ – Nick Jul 23 '15 at 19:16
  • \$\begingroup\$ GNU's glibc was non-conforming in this aspect until recently. Only in version 5.1 they changed from CoW to SSO strings. Before that they provided separate type vstring with SSO. \$\endgroup\$ – Ilya Popov Jul 23 '15 at 19:23
  • \$\begingroup\$ yep, gcc it is 5.1 \$\endgroup\$ – Nick Jul 23 '15 at 19:32
  • \$\begingroup\$ You might want to have a look at this SO question. There are two links to existing implementations (although I have not had a look at these myself, and don't know if it's what are you looking for) \$\endgroup\$ – Ilya Popov Jul 23 '15 at 19:41
  • \$\begingroup\$ And here is another implementation: cpp_immutable_string \$\endgroup\$ – Ilya Popov Jul 23 '15 at 19:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.