CODEWITHSUNDEEP
c++operator-overloadingoverloading
Rocketq
Rocketq

Reputation: 5781

Overloading operator +. String class

Im writing string class by myself. And I overloaded + operator. Its works fine, but then I tried to eguate cstr = str +pop , its did nothing. `You could see my error in main() function. Complier doesnt give any mistake.

#include <iostream>
#include <string.h>
#include <stdlib.h>

using namespace std;

class S {

public:
          S();
          S(const char *str);
          S(const S &s);

         ~S() { delete []string;}
          S  &operator  =(const S   &s);

          int  lenght() const {return l     ;}
      char*  strS() const {return string;}

      friend ostream &operator <<(ostream &, const S &first) {cout<<first.string;}
      friend S    operator+ (const S& first, const S& second);

private:
          char *string;
          int l;

};

int main(){
S pop("Q6");
S str("M5");

S cstr = str +pop; // works correct
cout<<str;

str = str + pop;
cout<<str ;        // doesnt work, it doesnt write in terminal

return 0;
}
S::S()
{
    l = 0;
    string = new char[1];
    string[0]='\0';
}

S::S(const char *str)
{
    l      = strlen(str);
    string = new   char[l+1];
    memcpy(string, str, l+1);
}

S::S(const S &s)
{
     l = s.l;
     string = new char[l+1];
     memcpy(string,s.string,l+1);
}

S &S::operator=(const S &s)
{
    if (this != &s)
    {
        delete []string;
        string = new char[s.l+1];
        memcpy(string,s.string,s.l+1);
        return *this;
    }
    return *this;
}

S    operator +(const S& first, const S& second)

{
    S temp;
    temp.string = strcat(first.strS(),second.strS());
    temp.l      = first.lenght() + second.lenght();

  return temp;
 }

I`m looking forward to your help.

Upvotes: 1

Views: 4962

Answers (5)

James Kanze
James Kanze

Reputation: 153899

Check the description of strcat. It appends the second argument to the first, supposing both are null terminated strings, and returns the first argument. In your case:

  • it appends to the string member of first, although there isn't enoguh memory for it (undefined behavior), and

  • it sets the string pointer in temp to point to the same memory as that in first; the first one to be destructed leaves the other pointing to deleted memory, and the memory allocated in the default constructor of temp is leaked.

Also, you never terminate your strings with '\0', so strcat may do just about anything.

A better solution would be to implement += first, and define + in terms of it. += would have to grow the memory it has, and append the text from the second string to it.

And while I'm at it: your operator= doesn't work either. It will leave the object in a state where it cannot be destructed if the new fails (throwing std::bad_alloc). You must ensure that all operations that can fail occur before the delete. (The fact that you need to test for self assignment is a warning sign. It's very rare for this test to be necessary in a correctly written assignment operator.) In this case, the swap idiom would probably be your best bet: copy construct a new S in a local variable, then swap their members.

Upvotes: 0

PiotrNycz
PiotrNycz

Reputation: 24347

Your operator has bugs!

S temp;
//^^^^ has only one byte buffer!!!
temp.string = strcat(first.strS(),second.strS());
//   1 byte   ^^^^^ strcat appends second.strS to first.strS

You should re-allocate memory for temp:

S temp;
temp.l      = first.lenght() + second.lenght();
delete [] temp.string; // !!!! - 
temp.string = new char[temp.l + 1]; // !!!!
// you should have another c-tor which can allocate memory!!!
// like: S(unsigned length, unsigned char c = '\0') 
strcpy(temp.string, first.strS());
strcat(temp.string, second.strS());

Besides this obvious bug - you should also take care of exceptions - std::bad_alloc for example. Look at copy-and-swap idiom for better approach for this task.

Upvotes: 3

john
john

Reputation: 8027

The problem is that your operator+ doesn't allocate any memory for the combined string. Nor does it copy the string to right place (it copies the string to first, not to temp). There's no easy fix with the class design you have.

Upvotes: 1

Alexander Chertov
Alexander Chertov

Reputation: 2108

The problem is with your implementation of operator+. strcat() appends the string ponted by the second argument to the string pointed by the first argument. The return value is the first argument. Therefore on return from operator+ the resulting S and the first S argument will be pointing to the same buffer. Which will later be deleted twice...

Upvotes: 0

Tom W
Tom W

Reputation: 1334

From the manpage for strcat:

 The strcat() and strncat() functions append a copy of the null-terminated
 string s2 to the end of the null-terminated string s1, then add a termi-
 nating `\0'.  The string s1 must have sufficient space to hold the
 result.

You're using it as if it allocates room for a new char array, then fills it. But, it doesn't do that.

Upvotes: 2

Related Questions