String class for general purpose library

I'm creating my own general purpose library for personal use in C++ and it needs a string class that fits in with the rest of the library. Currently, it only has a few basic features, but because strings are a pretty important thing, I want my class to be very optimized. I don't want to be writing a bunch of methods for a string model that doesn't work very well, so please leave some feedback so I can change things before it's too late.

common.h:

#ifndef _COMMON
#define _COMMON

namespace chaos {
    typedef long long llong;
    typedef unsigned char uchar;
    typedef unsigned short ushort;
    typedef unsigned int uint;
    typedef unsigned long long ulong;
    typedef signed char schar;
    typedef signed short sshort;
    typedef signed int sint;
    typedef signed long long slong;
}

#endif

string.h:

#ifndef _STRING
#define _STRING

#include "common.h"

namespace chaos {

    class String {
    private:
        char* str;
        ushort len;
    public:
        String();
        String(ushort);
        String(const char*);
        String(const String&);
        String(const String&, ushort, ushort);
        ~String();

        bool operator==(const String&);
        bool operator!=(const String&);
        String& operator=(const String&);
        String& operator+=(const String&);
        String operator+(const String&);

        char* getStr() const;
        ushort getLen() const;

        void set(ushort, char);
        char get(ushort) const;

        template static String From(T value, ushort base);
        template T to() const;
    };

}

#endif

string.cpp:

```
#include "string.h"

namespace chaos {

String::String() {
this->len = 0;
this->str = new char[this->len + 1];
}

String::String(ushort length) {
this->len = length;
this->str = new char[this->len + 1];
for (ushort i = 0; i len; i++)
this->str[i] = '.';
this->str[this->len] = '\0';
}

String::Stri

The DRY principle

Don't repeat yourself. If you find that you are writting the same piece of code again and again then you should probably pull that code into its own function.

I keep seeing this:

this->str = new char[this->len + 1];
        for (ushort i = 0; i len; i++)
            this->str[i] = /*STUFF*/;

Constructor chaining

C++ (since C++11) allows constructor chaining. This may help a lot with drying your code. Especially since most of those constructors do the same thing.

Initializing Elements.

You are obviously adding the extra byte onto your strings to make them null terminated. But you never set that value (you seem to assume that dynamically allocated memory will be zero'd. Take not it is not. The value of the memory is random and reading from it before initialization is UB.

String::String() {
    this->len = 0;
    this->str = new char[this->len + 1];  // So a size of 1!
    // But that single block of memory is not initialized. So you can't
    // assume that it contains the value '\0'
}

Use of this->

This is very uncommon in C++. I my opinion this is also bad practice as it hides real problems that would otherwise be spotted by the compiler.

The only real reason to do this is because you have a naming conflict and a local variable is shadowing a member variable. If this is true you should think of better names for your local variables. Shadowing can be detected by the compiler and thus prevent accidental assignment to the wrong variable.

On the other hand if you explicitly use this-> everywhere you don't turn on the check for shadowing (as most of your local variables now shadow members). So if you accidently forget to use this-> then you assign to the local and not the member and the compiler can not catch this type of error.

Const member functions

Members that do not mutate the state should be marked as const.

In a lot of C++ code parameters are passed as const references to a function to prevent copying. When you have a const reference to a member you are only allowed to call const member functions.

bool String::operator==(const String& string) const {
                                                  ^^^^^

Assignment

The assignment operator should provide the strong exception guarantee. If anything fails then the object should remain unchanged. Your assignment operator fails.

String& String::operator=(const String& string) {
        this->len = string.len;
        delete[] this->str;                    // str now points at invalid memory
        this->str = new char[this->len + 1];   // if new fails then an exception
                                               // is thrown. Leaving your object
                                               // in an invalid state if the
                                               // exception is caught.
        for (ushort i = 0; i len; i++)
            this->str[i] = string.str[i];
        return *this;
    }

The standard technique to get around this problem is called the copy and swap idiom.

String& String::operator=(String string)
{
    string.swap(*this);
    return *this;
}

I have written in detail about it in my blog.

Memory Leaking

You dynamically allocate memory in operator+ then pass it to an interface that does not accept ownership chaos::String::String(). As a result your function leaks memory.

String String::operator+(const String& string) {
        char* str = new char[this->len + string.len + 1];  // dynamically allocated.

        // STUFF

        return String(str);  // Passed to constructor.
                             // But that constructor does not take ownership.
                             // Instead another copy is made. So this
                             // memory is leaked.
    }

Normally the operator? is implemented in terms of operator?=. It may not be the most efficient implementation but it usually works.

String String::operator+(String const& string) const
{
    String tmp(*this);
    return tmp += string;
}

This was wrong:

Explicit template expansion

template String String::From(char, ushort);
    template String String::From(short, ushort);
    template String String::From(int, ushort);
    template String String::From(llong, ushort);
    template String String::From(uchar, ushort);
    template String String::From(ushort, ushort);
    template String String::From(uint, ushort);
    template String String::From(ulong, ushort);
    template String String::From(schar, ushort);
    template String String::From(sshort, ushort);
    template String String::From(sint, ushort);
    template String String::From(slong, ushort);

This will not prevent other types from being valid in From. To do that you must separate the declaration and definition. See here

Underscore in Identifiers

Best to avoid. Especially if you don;t know the rules. But you have an error:

#ifndef _COMMON
#define _COMMON

The identifie

StackExchange Code Review Q#141955, answer score: 2