the operator "="error in class "String"

Code:

---

#include <iostream>
using namespace std;
class String
{
public:
        String(const char *str = NULL); // 通用构造函数
        String(const String &another); // 拷贝构造函数
        ~ String(); // 析构函数
        String& operator =(const String&);// 赋值函数
        friend ostream& operator << (ostream&, const String& s);
        friend istream& operator>>(istream& in, String& s);
        friend String& operator+(const String&,const String&);
private:
        char *m_data; // 用于保存字符串
};
String::String(const char *str)
{
  if (str == NULL)
      m_data = NULL;
  else
  {
          m_data = new char[strlen(str)+1];
      strcpy(m_data,str);
  }

}
String::String(const String &another)
{
  m_data = new char[strlen(another.m_data)+1];
  strcpy(m_data,another.m_data);
}
String::~String()
{
        if(m_data != NULL)
        {
                delete m_data;
                m_data = NULL;
        }
}
String & String::operator=(const String &rhs)
{
  if (*(this->m_data) == *rhs.m_data)
      return *this/*rhs*/;
  if(this->m_data!=NULL)
  this->~String();
  m_data = new char[strlen(rhs.m_data)+1];
  strcpy(m_data,rhs.m_data);
  return *this;
}
ostream& operator << (ostream& out, const String& s)
{
  if(s.m_data)
  out << s.m_data << endl;
  return out;
}
istream& operator>>(istream& in, String& s)
{
        char *s1 = new char[1000];
  in >> s1;
  s.m_data = new char[strlen(s1)+1];
  strcpy(s.m_data,s1);
  return in;
}
String& operator+(const String& lhs,const String&rhs)
{
  String ret(lhs);
  strcat(ret.m_data,rhs.m_data);
  return ret;
}
int main()
{
  String s ;
  cin >> s;
  String s1;
  cin >> s1;
  String s2= s+s1;//this sentence has question , but i don't know why
  cout << s2;
}

---

who can explain it to me ?thanks!

On running the program, the error is trying to return a local variable in the overload + function...what I got here
You are returning a local object..which is wrong

>> if(m_data != NULL)
Deleting NULL is perfectly defined. You can remove that.

>> if (*(this->m_data) == *rhs.m_data)
Very bad. Checks if the first character in the strings are equal. You should check for self-assignment, which is usually

if (this == &rhs) or
if (*this == rhs)

>> char *s1 = new char[1000];
>> in >> s1;
Never do this. You can get a buffer overrun.
Read into a std::string with std::getline or use I/O manipulators to limit the length of data to read.
Example:

std::string s;
std::getline(std::cin, s);

Or

std::string s;
std::cin >> s;

operator >> is broken; it leaks memory.
Suggested approach:

std::string s;
stream >> s;
m_data = new char[s.size() + 1];
strcpy(m_data, s.c_str());

operator + is broken in design. It should return a temporary, not a reference. Furthermore, it doesn't check for enough room in the buffer, so buffer overflows can occur.

You also failed to mention what the problem was.

Quote:

---

Originally Posted by Elysia

>> char *s1 = new char[1000];
>> in >> s1;
Never do this. You can get a buffer overrun.
Read into a std::string with std::getline or use I/O manipulators to limit the length of data to read.
.

---

Wouldn't using std::string defeat the purpose of making a string class in the first place? ....
If it is an exercise; he won't probably be allowed to use std::string ..

It is a solution; I didn't mention it was the only solution.

Don't implement your operator= that way. It is not exception safe.
Instead use the "copy and swap" idiom. It is much easier, is exception safe, and may solve whatever error you were getting. Speaking of which, how about posting the actual error huh?!

Quote:

---

Originally Posted by Elysia

operator >> is broken; it leaks memory.
Suggested approach:

std::string s;
stream >> s;
m_data = new char[s.size() + 1];
strcpy(m_data, s.c_str());

---

Seems like you fixed the buffer overrun but did nothing about the leak. To be clear, the memory that m_data held before using operator>> would be leaked, because m_data was not released before assigning new memory.

Quote:

---

Originally Posted by Elysia

Read into a std::string with std::getline or use I/O manipulators to limit the length of data to read.

---

Just to be clear, the IO manipulator that works for this is called std::setw.

Quote:

---

Originally Posted by manasij7479

Wouldn't using std::string defeat the purpose of making a string class in the first place? ....
If it is an exercise; he won't probably be allowed to use std::string ..

---

if don't use std::string ,how to write ?

I explained in my post so you probably want to read the thread. All you need to do to know about std::setw and see examples is look it up. If you cannot use manipulators, and cannot use std::string, then the best you could do is not use operator>> for this, and instead use the istream::get() function.

Quote:

---

Originally Posted by whiteflags

I explained in my post so you probably want to read the thread. All you need to do to know about std::setw and see examples is look it up. If you cannot use manipulators, and cannot use std::string, then the best you could do is not use operator>> for this, and instead use the istream::get() function.

---

ok！thank you！