3

I started writing a very simple implementation of a string class in c++, here is the code:

class String
{
public:
    String()
    {
        this->_length = 0;
        this->_size = 0;
        this->_string = NULL;
    }
    String(const char* str)
    {
        this->_length = strlen(str);
        this->_size = this->_length + 1;
        this->_string = new char[this->_size];

        strcpy_s(this->_string, this->_size, str);
    }
    ~String()
    {
        if (this->_string != NULL)
        {
            delete[] this->_string;
            this->_string = NULL;
        }
    }

    String& operator+(const char* str)
    {
        String* temp = new String();

        temp->_length = strlen(str) + strlen(this->_string);
        temp->_size = temp->_length + 1;
        temp->_string = new char[temp->_size];

        strcpy_s(temp->_string, temp->_size, this->_string);
        strcat_s(temp->_string, temp->_size, str);

        return (String&)*temp;
    }

    int Length()
    {
        return this->_length;
    }

private:
    int _size;
    int _length;
    char* _string;
};

You can see that my implementation of operator+ is absolutely wrong, in fact there is a memory leak. Writing the operator+= is way simpler because I can simply concatenate the char* with this->_string and return *this. I need help with the operator+ implementation.

Note: This is homework so I don't want the solution the copy-paste but it would be awesome if someone could point me in the right direction...

Thanks

Edit: I added the copy constructor:

String(const String& str)
{
    this->_length = str._length;
    this->_size = str._size;
    this->_string = new char[this->_size];
    strcpy_s(this->_string, this->_size, str._string);
}

the operator= and the operator+=:

String& operator=(const String& str)
{
    if (this != &str)
    {
        this->_length = str._length;
        this->_size = str._size;
        this->_string = new char[this->_size];

        strcpy_s(this->_string, this->_size, str._string);
    }

    return *this;
}
String& operator+=(const String& str)
{
    this->_length = this->_length + str._length;
    this->_size = this->_length + 1;

    char* buffer = new char[this->_size];
    strcpy_s(buffer, this->_size, this->_string);
    strcat_s(buffer, this->_size, str._string);

    delete[] this->_string;
    this->_string = buffer;

    return *this;
}

but there is still something wrong because if I run a while(true) loop like this:

while (true)
{
    String a = String("string a");
    String b = a;
    b = "string b";
    b += " string c";
}

the memory used by the process will increase continuously

Improve this question
10
  • 5
    temp doesn't need to be a pointer. Make it an automatic variable. You also should not return by reference in that case. Commented May 12, 2013 at 20:27
  • 1
    @0x499602D2 shouldn't you make that an answer? Commented May 12, 2013 at 20:28
  • @Named There are also other problems. Commented May 12, 2013 at 20:29
  • Pls look at this - daniweb.com/software-development/cpp/threads/410105/… Commented May 12, 2013 at 20:30
  • 4
    Also need to follow "rule of three". Commented May 12, 2013 at 20:31

2 Answers 2

Reset to default
7

You could reuse the operator+= in the operator+:

(The code below assumes that you have an operator+=, a copy constructor and an assignment operator which is NOT the case in the code you pasted).

EDIT: As suggested by Jerry Coffin the following operator should NOT be a class member but a free operator:

EDIT2: And to allow the compiler a bit more optimizations the first argument is not a const-reference anymore:

String operator+(String a, String const &b) {
  a += b;
  return a;
}

By this you can have a more simple operator+= and simply copy constructor and build the complex thing on the simple ones.

And do not forget:

You MUST implement a copy constructor and assignment operator. Otherwise the compiler generates it for you in a wrong way: The compiler generates code that simply copies the content. So it also copies the pointer but does not allocate new memory for the copy. Then you have two instances referencing the same memory and both try to deallocate it in the destructor which is undefined behavior.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

but he still need copy constructor and assignment operator
@Sergi0: Yes, he needs...but he needs in any way since otherwise his implementation is wrong (i.e. it would use the automatic assignment operator and copy constructor that just copies the pointer which leads to double de-allocation).
and strlen isn't safe to use without checking for NULL
@Sergi0: I do not want to evaluate the remaining code pasted in the original question but only try to answer the question.
3

One easy way to implement your operator+ is in terms of +=. Create a copy of the left operand, then use += to add the right operand to it, and finally return the result.

Also note that operator+ shouldn't usually be a member function -- it should normally be a free function. The basic difference is that as a member function, the left operand must already be a string for it to work. As a free function, your string constructor can be used to convert (for example) a string literal. For example, string+"something"; will work with a member function, but "something" + string; won't. With + overloaded as a free function, both of them will work.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.