# C++ String implementation

I'm looking to get some feedback on my attempted string implementation. The code is just meant for me to play around with and work on some C++ to try and improve. A lot of the things done here (rule of 4 and 1/2, auto return values, const methods, etc) are features of C++ I don't have a lot or any experience with and I want to get used to them and understand when to use them.

class string
{
public:
explicit string(const char *array = nullptr) :
cString(array ? new char[size]: nullptr), size(array ? strlen(array) + 1: 0), capacity(size)
{
std::copy(array, array + size, cString);
}

string (string& other) :
cString(other.cString ? new char[size] : nullptr), size(other.cString ? strlen(other.cString) + 1 : 0), capacity(size)
{
std::copy(other.cString, other.cString + other.size, cString);
}

string(string&& other) noexcept : string()
{
swap(*this, other);
}

string& operator=(string& other) noexcept
{
swap(*this, other);

return *this;
}

friend std::ostream& operator<<(std::ostream& output, const string& str) noexcept
{
output << str.cString;

return output;
}

inline bool operator==(const string& str) const noexcept
{
return strcmp(cString, str.cString) == 0;
}

inline char operator[](const std::size_t index) const
{
if (index > size) {
throw std::invalid_argument("Index out of bounds");
}

return *(cString + index);
}

inline bool operator!=(const string& str) const noexcept
{
return !(*this == str);
}

string& operator +=(const char *str)
{
if (!str) {
throw std::invalid_argument("Null pointer");
}

std::size_t stringLen = strlen(str);

if (size + stringLen > capacity) {
std::size_t newSize = (size + stringLen + capacity) * 2;
char *newBuffer = new char[newSize];

std::copy(cString, cString + size - 1, newBuffer);
std::copy(str, str + stringLen + 1, newBuffer + size - 1);

delete[] cString;
cString = newBuffer;
capacity = newSize;
} else {
std::copy(cString, cString + size - 1, cString);
std::copy(str, str + stringLen + 1, cString + size - 1);
}

size = size + stringLen;

return *this;
}

friend void swap(string& first, string& second) noexcept
{
using std::swap;

swap(first.size, second.size);
swap(first.cString, second.cString);
}

void clear() noexcept
{
memset(cString, 0, size);
size = 0;
}

static void copy(const string& src, string& dst)
{
if (src.capacity > dst.capacity)
{
char *newBuffer = new char[src.size];

std::copy(src.cString, src.cString + src.size, newBuffer);

delete[] dst.cString;
dst.cString = newBuffer;
dst.size = src.size;
} else {
std::copy(src.cString, src.cString + src.size, dst.cString);
}

}

void replace(const char oldChar, const char newChar) const
{
for(std::size_t i = 0; i < size - 1; i++) {
if (*(cString + i) == oldChar) {
*(cString + i) = newChar;
}
}
}

decltype(auto) hash() const noexcept
{
unsigned long value = 0;
int character;

for (std::size_t i = 0; i < size - 1; i++) {
character = *(cString + i);
value = character + (value << 6) + (value << 16) - value;
}

return value;
}

inline bool isEmpty() const noexcept
{
return size == 0;
}

inline char front() const noexcept
{
return *cString;
}

inline char back() const noexcept
{
return *(cString + size - 2);
}

inline char *getArray() const noexcept
{
return cString;
}

inline decltype(auto) getSize() const noexcept
{
return size;
}

inline decltype(auto) getCapacity() const noexcept
{
return capacity;
}

~string()
{
delete [] cString;
}
private:
std::size_t size;
std::size_t capacity;
char *cString;
};

• Welcome to Code Review! To clarify, can you give a bit more detail about what your code does or is for, exactly? Is it meant to replicate std::string to some extent, or is it something completely different? – Phrancis Aug 22 '16 at 0:57
• Thanks! The code is just meant for me to play around with and work on some c++ to try and improve. A lot of the things done here (rule of 4 and 1/2, auto return values, const methods, etc) are features of c++ I don't have a lot or any experience with and I want to get used to them and understand when to use them. – FrozenHawk Aug 22 '16 at 1:08
• May I suggest that you edit your post to add some of this information, then? As it stands it's kind of vague. This will help reviewers give you better answers. – Phrancis Aug 22 '16 at 1:13

1. Counting the 0-terminator as part of the string's length is a very questionable design-decision, considering it is quite unique and has interesting consequences for joining strings.
2. There are null strings and empty strings with the current design, which is a nice source of errors.
3. The copy-ctor should accept a constant reference. Also, it should not recalculate the sources size instead of just copying it.
4. The assignment-operator is broken, as it does a swap. The quick and easy fix would be simply accepting the argument by value.
5. The stream-inserter should use iostream.write for performance and to handle internal 0-bytes.
6. operator== should properly handle null strings, or preferably they should be eliminated.
7. operator[] must throw if the index equals the size, because of the curious decision to count the terminator too.
8. operator+= has a spurious copy string on itself in each branch. Also, there should be the full complement of operators, especially also accepting your custom string-type itself.
9. swap is broken because you fail to swap capacities.
10. clear is wasteful, and creates a chimera of null and empty string.
11. copy is useless, just use the copy-ctor.
12. You know we have std::replace?
13. Only use return-type-deduction with decltype(auto) instead of auto if you want to return a reference. A tip, it's an error everywhere you used it.
• Thanks for the feedback. Could you expand on why return-type-deduction should only be used to return a reference? – FrozenHawk Aug 23 '16 at 14:20
• @FrozenHawk There are two possibilities for type deduction: auto (never a reference) and decltype(auto) (can be a reference). You want the former. – Deduplicator Aug 23 '16 at 14:39

## Subscript operator

Your range-checking allows access to one character beyond the string's buffer. I think you want if (index >= size). Also, when using the subscript operator, I think it is expected you are getting a reference into the container. So it should return (in your example) a char const&. You should also define a non-const version when indexing into a non-const string

char& operator[] (


## Assignment Operator

Usually the assignment operator promises not to modify the source:

string& operator=(string const&) noexcept;


Your assignment operator changes the value of what was assigned. This will confuse your users.