I have overloaded << and = operators. Why when I assign an object to another and try to print it I get junk printed out?

Question

I have a program with a class Length. This class has an attribute named size of type int and a dynamic array *numb of type char. I have overloaded operators << and = so that I can print object values and assign values of an object to another.

If I leave the return type of operator = to void, the program seems to work fine but if I try to return a Length object I get junk printed. Why?. Thank you.

Here is my code:

class Length
{
    int size;
    char *numb;
public:
    Length()
    {

    }
    Length(int size, char *n);
    ~Length();
    friend ostream & operator << (ostream &channel, const Length &l);
    Length operator = (const Length &x);

};


Length::Length(int size, char  *n)
{
    this->size = size;
    numb = new char[size];

    for(int i = 0; i < size; i++)
    {
        numb[i] = n[i];
    }
}

ostream & operator << (ostream &channel, const Length &l)
{
    channel << "Size " << l.size <<endl;
    for(int i = 0; i < l.size; i++)
    {
        channel << l.numb[i] << endl;
    }
    return channel; 
}

Length Length::operator =(const Length &x)
{
    
    delete [] this->numb; 
    this->size = x.size;
    this->numb = new char[this->size];
    memcpy(this->numb, x.numb, this->size);
    return *this; //If I comment that line and make return type void programm works fine
}


int main()
{
    char * ch = "Hello";
    char * cx = "Hi";

    int size = strlen(ch);
    int size_x = strlen(cx);

    Length h(size, ch);
    Length x(size_x, cx);
    cout << x; //Works fine
    cout << h <<endl; //Works fine
    x = h; 
    cout << x <<endl; //Prints junk
}

Read about "the rule of three" and then add a copy constructor. (Somewhat related to that problem: `operator=` is expected to return a reference, not a copy ) — molbdnilo, Jun 06 '20 at 16:19
@Yksisarvinen I was getting random charachters, like numbers and such. But if I return a reference works fine. — C96, Jun 06 '20 at 16:29
This doesn't address the question, but you really don't need the extra stuff that `std::endl` does. Use `'\n'` to end a line. — Pete Becker, Jun 06 '20 at 19:01

john · Answer 1 · 2020-06-06T16:33:50.060

To expand on what molbdnilo said, there are two problem with your code. the first problem is that operator= should return a reference. It's not an error when it doesn't but it results in behaviour that is inconsistent with how assignment normally behaves. I'm not going into the details here, you can look them up if you like.

But when combined with your second error you do get problems. Because you are returning Length by value from operator= you invoke the Length copy constructor. But your class doesn't have a copy constructor so it uses the default, and that default does the wrong thing. What happens is that when you return from your operator= the this->numb pointer gets copied to the temporary object that is the return value of operator=. That temporary object then gets destroyed, the consequence of that is that the memory that this->numb is pointing at gets deleted. And that is why you see garbage when you print out x, because it's internal memory has been freed.

Two fixes are possible, return a reference from operator=, and write a valid copy constructor for your class. Either would fix the problem, you should do both.

And you should also read up on the rule of three, to fully understand this very important issue.

@Yksisarvinen It's a fair question, and a complicated topic because I believe a precise answer depends on the version of C++ being used. I don't know all the details. But I think it's clear that this is what is happening in the OPs case, if it wasn't then they wouldn't see an error. — john, Jun 06 '20 at 16:38

NixoN · Accepted Answer · 2020-06-06T16:43:55.407

1

You should return here reference:

Length& Length::operator =(const Length &x)
{
    if (this != &x)
    {
         delete [] this->numb; 
         this->size = x.size;
         this->numb = new char[this->size];
         memcpy(this->numb, x.numb, this->size);
    }
    return *this; 
}

And add copy constructor:

Length(Length& len) 
{
    size = len.size;
    numb = new char[size];
    for (int i = 0; i < size; ++i)
    {
         numb[i] = len.numb[i];
    } 
}

edited Jun 06 '20 at 16:43

answered Jun 06 '20 at 16:29

NixoN

661
1
6
19

@Costas96 Incidentally, your assignment operator has another problem. It would fail in the case of self assignment, `x = x;`. You might also want to read up on the [copy and swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) which is the generally recommended way of writing an assignment operator – john Jun 06 '20 at 16:41

I have overloaded << and = operators. Why when I assign an object to another and try to print it I get junk printed out?

2 Answers2