Concatenate 2 string using operator+= in class C++

Question

I've seen some similar questions before asking, but I'm still stuck at the part of concatenating two strings using operator+=.

Currently, I can get separate strings correctly by constructor method. But when I compile code, the line str[length+i] = s[i]; in the method String& String::operator+= (const String& s) shows an error:

no match for ‘operator[]’ (operand types are ‘const String’ and ‘unsigned int’)

So I need your help to fix this bug.

Here's my code:

#include <iostream>
#include <cstring>
 
class String {
 
    // Initialise char array
    char* data;
    unsigned length;
 
public:
    // Constructor without arguments 
    String();
 
    // Constructor with 1 arguments
    String(char* s);
 
    // Copy Constructor
    String(const String& source);
 
    // Move Constructor
    String(String&& source);
 
    // Destructor
    ~String() { delete[] data; }
    
    /*! 
     *  @brief String length.
     *  @return Value in String @c length.
     */
    unsigned len ( ) const;
    
    /*! 
     *  @brief Append to String.
     *  @param[in] s A String object.
     *  @return A String reference to *this.
     *  @post String will equal the concatenation of itself with @a s.
     */
    String& operator+= (const String& s);
};
 
// Constructor with no arguments
String::String()
    : data{ nullptr }
{
    data = new char[1];
    data[0] = '\0';
}
 
// Constructor with one arguments
String::String(char* s)
{
    if (s == nullptr) {
        data = new char[1];
        data[0] = '\0';
    }
 
    else {
 
        data = new char[strlen(s) + 1];
 
        // Copy character of s[]
        // using strcpy
        strcpy(data, s);
        data[strlen(s)] = '\0';
 
        std::cout << data << "\n";
    }
}
 
// Copy Constructor
String::String(const String& source)
{
    data = new char[strlen(source.data) + 1];
    strcpy(data, source.data);
    data[strlen(source.data)] = '\0';
}
 
// Move Constructor
String::String(String&& source)
{
    data = source.data;
    source.data = nullptr;
}

unsigned String::len ( ) const 
{
    return length;
}

String& String::operator+= (const String& s) 
{
    unsigned len = length + s.len();
    char*    str = new char[len];

    for (unsigned j=0; j < length; j++)
        str[j] = data[j];

    for (unsigned i=0; i < s.len(); i++)
        str[length+i] = s[i];

    delete data;
    length = len;
    data   = str;
    return *this;    
}
 
int main()
{
    // Constructor with no arguments
    String a;
 
    // Convert string literal to
    // char array
    char temp[] = "Hello world.";
 
    // Constructor with one argument
    std::cout << "s1: ";
    String s1{ temp };
 
    // Copy constructor
    String s11{ a };
 
    char temp1[] = "Goodbye!";
    std::cout << "s2: ";
    String s2{ temp1 };
    
    String s3 = String s1 + String s2;

    return 0;
}

Another way of writing main function:

int main()
{
String s1("Hello World.");
String s2("Goodbye!");
std::cout << "s1: " << s1 << std::endl;
std::cout << "s2: " << s2 << std::endl;
String s3 = s1 + s2;
std::cout << "s3: " << s3 << std::endl;
std::cout << "The last char of s3: " << s3[s3.size()-1] << std::endl;
return 0;
}

Expected result:

s1: Hello World.
s2: Goodbye!
s3: Hello World.Goodbye!
The last char of s3: !

How can I modify my code to get s3 and last char of s3 correctly?

Answer 1

In many of your constructors, you do not set length which leaves it with an indeterminate value - and reading such values makes the program have undefined behavior. So, first fix that:

#include <algorithm> // std::copy_n

// Constructor with no arguments
String::String() : data{new char[1]{'\0'}}, length{0} {}

// Constructor with one argument
String::String(const char* s) {     // note: const char*
    if (s == nullptr) {
        data = new char[1]{'\0'};
        length = 0;
    } else {
        length = std::strlen(s);
        data = new char[length + 1];
        std::copy_n(s, length + 1, data);
    }
}

// Copy Constructor
String::String(const String& source) : data{new char[source.length + 1]},
                                       length{source.length}
{
    std::copy_n(source.data, length + 1, data);
}

// Move Constructor
String::String(String&& source) : String() {
    std::swap(data, source.data);
    std::swap(length, source.length);
}

In operator+= you are trying to use the subscript operator, String::operator[], but you haven't added such an operator so instead of s[i], use s.data[i]:

String& String::operator+=(const String& s) {
    unsigned len = length + s.length;
    char* str = new char[len + 1];
    for (unsigned j = 0; j < length; j++) str[j] = data[j];
    for (unsigned i = 0; i < s.length; i++) str[length + i] = s.data[i];
    str[len] = '\0';
    delete[] data;        // note: delete[] - not delete
    length = len;
    data = str;
    return *this;
}

If you want to be able to use the subscript operator on String objects, you would need to add a pair of member functions:

class String {
public:
    char& operator[](size_t idx);
    char operator[](size_t idx) const;
};
char& String::operator[](size_t idx) { return data[idx]; }
char String::operator[](size_t idx) const { return data[idx]; }

And for String s3 = s1 + s2; to work, you need a free operator+ overload:

String operator+(const String& lhs, const String& rhs) {
    String rv(lhs);
    rv += rhs;
    return rv;
}

Also, to support printing a String like you try in your alternative main function, you need an operator<< overload. Example:

class String {
    friend std::ostream& operator<<(std::ostream& os, const String& s) {
        os.write(s.data, s.length);
        return os;
    }
};

Full demo