C++ String implementation

Question

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;
};

• Phrancis: 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?
• FrozenHawk: 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.
• Phrancis: 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.

Answer 1

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.

• FrozenHawk: Thanks for the feedback. Could you expand on why return-type-deduction should only be used to return a reference?
• Deduplicator: @FrozenHawk There are two possibilities for type deduction: auto (never a reference) and decltype(auto) (can be a reference). You want the former.

Answer 2

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.