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?

CodePudding user response：

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

// 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(s, s   length   1, data);
    }
}

// Copy Constructor
String::String(const String& source) : data{new char[source.length   1]},
                                       length{source.length}
{
    std::copy(source.data, 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;
}

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

CodePudding user response：

For starters neither constructor sets the data member length. So the operator

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

has undefined behavior.

Also provided that the data member length was initialized you need to write

    char*    str = new char[len   1];

instead of

    char*    str = new char[len];

to reserve memory for the terminating zero character '\0' because you are using the standard C string function strcpy in the copy constructor

strcpy(data, source.data);

And the class does not have the subscript operator used in this for loo[

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

And you forgot to append the terminating zero character '\0'.

Pay attention to that there is no member function size in the class used in this expression

s3[s3.size()-1]

And this construction

String s3 = String s1   String s2;

is invalid. At least you should write

String s3 = s1 s2;

and correspondingly define the operator .