-1

I'am trying to make my own "String" class. But i have problems with overloading operator+. I made operator += that works good and friend operator+ that sometimes does not work as i plan.

String()
{
    length = 0;
    p = new char[1];
    p[0] = '\0';
}
String(const char*s)
{
    length = strlen(s);
    p = new char[length+1];
    strcpy(p, s);
}
String(const String& s)
{
    if (this != &s)
    {
        length = s.length;
        p = new char[s.length + 1];
        strcpy(p, s.p);
    }
}
~String()
{
    delete[]p;
};
String &operator+=(const String&s1)
{
    String temp;
    temp.length = length + s1.length;
    delete[]temp.p;
    temp.p = new char[temp.length + 1];
    strcpy(temp.p, p);
    strcat(temp.p, s1.p);
    length = temp.length;
    delete[]p;
    p = new char[length + 1];
    strcpy(p, temp.p);
    return *this;
}
friend String operator+(const String &s1, const String &s2) 
{
    String temp1(s1);
    temp1 += s2;
    return temp1;
}

If i use operator + like this : String c =a+b; all works as planed,but if i write a=a+b; i get error String.exe has triggered a breakpoint. What should i correct? /////I solved problem overloading operator= Thanks!

Vladislav
  • 67
  • 8
  • Have you considered the case when `this` and `s1` are the same strings – Ed Heal May 01 '16 at 14:11
  • @EdHeal hm, i works correct when i use a+=a; – Vladislav May 01 '16 at 14:14
  • 1
    Change `temp1 += s1;` to `temp1 += s2;`. – songyuanyao May 01 '16 at 14:15
  • @songyuanyao same error – Vladislav May 01 '16 at 14:20
  • 3
    @songyuanyao You are right about the typo, but that does not explain the "breakpoint" (segfault, I suppose). Vladislav, can you please provide a [mcve] and show us more details about the error you're getting? Based on the limited information, my best guess is that the cause of your problem is outside of the code you're showing us. – mindriot May 01 '16 at 14:20
  • 1
    @EdHeal is correct. `strcat(p, s1.p)` fails for s1 == this since s1.p has already been deleted. – stark May 01 '16 at 14:25
  • 1
    @Vladislav You're supposed to provide a [MCVE] in your question. Just providing a pastebin link in a comment doesn't improve your question. Expect being it deleted when it stays in its current form. – πάντα ῥεῖ May 01 '16 at 14:27
  • @πάνταῥεῖ i have edited my post. I am providing now Minimal, Complete, and Verifiable example? – Vladislav May 01 '16 at 14:43
  • @mindriot i have added the information – Vladislav May 01 '16 at 14:45
  • @Vladislav Nope. That's still not a [MCVE]. See [here](http://coliru.stacked-crooked.com/a/3aee151517379931): – πάντα ῥεῖ May 01 '16 at 14:47
  • @stark The deletion is not the issue, as a new valid array has been put in it's place. The issue is calling strcat with the same pointer for both arguments. – Jfevold May 01 '16 at 15:31
  • You broke the Rule of Three: http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – Alan Stokes May 01 '16 at 15:55

1 Answers1

0
String operator+=(const String&s1)
{
    int temp_length = length + s1.length;
    String temp(*this);
    length = temp_length;
    delete[]p;
    p = new char[length + 1];
    strcpy(p, temp.p);
    strcat(p, s1.p); //<-- This is the problem
    return *this;
}

friend String operator+(const String &s1, const String &s2) 
{
    String temp1(s1);
    temp1 += s1;
    return temp1;
}

In the line marked above, you're accessing s1.p, which is the same as this.p in the situation you're describing as being the problem. Calling strcat with the same array for both parameters is strictly forbidden. You need to make another copy.

see this answer

According to strcat(3):

The strcat() function appends the src string to the dest string, overwriting the terminating null byte ('\0') at the end of dest, and then adds a terminating null byte. The strings may not overlap, and the dest string must have enough space for the result.

Suggested Solution:

String operator+=(const String&s1)
{
    int temp_length = length + s1.length;
    String temp(*this);
    length = temp_length;
    delete[]p;
    p = new char[length + 1];
    strcpy(p, temp.p);
    if(p == s1.p)
    {
        String other_temp(s1.p);
        strcat(p, other_temp);
    } else
        strcat(p, s1.p); //<-- no longer a problem
    return *this;
}

friend String operator+(const String &s1, const String &s2) 
{
    String temp1(s1);
    temp1 += s1;
    return temp1;
}
Community
  • 1
  • 1
Jfevold
  • 422
  • 2
  • 11
  • Thank you! I made like you and overloaded operator=. Now it works correctly – Vladislav May 01 '16 at 15:52
  • please then accept the answer, or if you still find it insufficient, let me know what you would need to consider it an acceptable answer. – Jfevold May 01 '16 at 15:53