0

I was checking assignment operator implementations, and I do not understand this:

const MyString& operator=(const MyString& rhs)
{ 
    if (this != &rhs) {
        delete[] this->str; // Why is this required?
        this->str = new char[strlen(rhs.str) + 1]; // allocate new memory
        strcpy(this->str, rhs.str); // copy characters
        this->length = rhs.length; // copy length
    }
    return *this; // return self-reference so cascaded assignment works
}

Why can't I just do this instead, without freeing memory then allocating new memory?

void operator=(const MyString& rhs)
{ 
    if (this != &rhs) {
        strcpy(this->str, rhs.str); // copy characters
        this->length = rhs.length; // copy length
    }
}

Why can't I just update the values in the existing memory?

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • How much of the existing memory do you have? Maybe it's too little? maybe it's too much? – n. m. could be an AI Jun 21 '21 at 21:21
  • 4
    `rhs` could be larger than the array allocated for the current sting content. Furthermore you may want to use the smallest amount of memory you're able to store the string in; You could of course introduce a member `capacity` which in some cases may allow you to reuse the old array. (`std::string` uses an approach similar to this) – fabian Jun 21 '21 at 21:21
  • An aside: It may not be optimal here, but the [Copy and Swap Idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) can turn writing even the hardest of assignment operators into an absolute cake walk. – user4581301 Jun 21 '21 at 21:29
  • 1
    *And* copy-and-swap will make it exception-safe. Think about what will happen if `new` fails. – HolyBlackCat Jun 21 '21 at 21:51

4 Answers4

2

The MyString being copied from can be a different length than the MyString being assigned to.

An array can't be resized. To create an array of a different size, you would have to destroy the old array and replace it with a new array. That is what the first code is doing.

In the second code, it makes sense to reuse the existing array only if the new array is smaller or equal in size, which you are not checking for, eg:

const MyString& operator=(const MyString& rhs)
{ 
    if (this != &rhs) {
        if (rhs.length > this->length) {
            delete[] this->str;
            this->str = new char[rhs.length + 1];
        }
        strcpy(this->str, rhs.str);
        this->length = rhs.length;
    }
    return *this;
}

In which case, you should consider adding another member capacity to better separate how many chars are physically allocated for the array versus how many chars are logically valid inside of the array, eg:

MyString()
    : str(NULL), length(0), capacity(0)
{
}

MyString(const MyString& src)
    : str(NULL), length(0), capacity(0)
{
    if (src.str) {
        this->capacity = rhs.length; // optionally round up to an even boundary of your choosing
        this->str = new char[this->capacity + 1];
        strcpy(this->str, src.str);
        this->length = rhs.length;
    }
}

const MyString& operator=(const MyString& rhs)
{ 
    if (this != &rhs) {
        if (rhs.length > this->capacity) {
            delete[] this->str;
            this->capacity = rhs.length;  // optionally round up to an even boundary of your choosing
            this->str = new char[this->capacity + 1];
        }
        strcpy(this->str, rhs.str);
        this->length = rhs.length;
    }
    return *this;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • You'd have to wonder if you should reuse a Megabyte allocation for a 1-character string. Perhaps you should reallocate if >100% or <50%. – MSalters Jun 21 '21 at 22:50
  • @MSalters Neither `g++` nor `clang++` reallocates even if you go from 1 MIB to an empty string. [example](https://godbolt.org/z/1o5P5bdxq) – Ted Lyngmo Jun 22 '21 at 15:22
  • @TedLyngmo: g++ has nothing to do with your own logic; this is `MyString` not `std::string`. (And even that is more accurately called libstdc++) – MSalters Jun 22 '21 at 16:45
  • @MSalters True. I just meant that it's probably ok to do what some of the most commonly used standard libraries do. At least per default. – Ted Lyngmo Jun 22 '21 at 16:52
  • thanks for your answers,, So, this approach is only to make sure the LHS have the same size as RHS, right ? – Ahmed Salama Jun 22 '21 at 20:38
  • @AhmedSalama The original code reallocates the array's memory unconditionally, to make the target `MyString` always have the same `length` as the source `MyString`. The suggestion is to perform that reallocation only when the source `MyString` is *larger* than the target `MyString`. If the target `MyString` already has sufficient memory allocated, reallocating is unnecessary overhead. – Remy Lebeau Jun 22 '21 at 20:43
0

why Just I can not update the values in the existing memory

You can do that if the LHS has enough memory. Otherwise, you'll have to deallocate old memory and allocate new memory.

const MyString& operator=(const MyString& rhs)
{ 
   if (this != &rhs) {
      if ( this->length < rhs.length )
      {
         this->length = rhs.length;
         delete[] this->str;
         this->str = new char[strlen(this->length) + 1];
      }
      strcpy(this->str, rhs.str);
   }
   return *this;
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
0

You only need to allocate new memory if the rhs string is longer than what you've already have allocated space for:

MyString& operator=(const MyString& rhs) {  // don't return a `const&`
    if (this != &rhs) {
        if(length < rhs.length) {
            delete[] str;
            str = new char[rhs.length + 1];
        }
        length = rhs.length;
        std::memcpy(str, rhs.str, length + 1);
    }
    return *this;
}

Note that this will loose the information about the extra space if you don't add a capacity member variable. With that added, it'd look like this:

MyString& operator=(const MyString& rhs) { 
    if (this != &rhs) {
        if(capacity < rhs.length) {
            delete[] str;
            capacity = rhs.length;
            str = new char[capacity + 1];
        }
        length = rhs.length;
        std::memcpy(str, rhs.str, length + 1);
    }
    return *this;
}

Demo

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
0

For starters the operator = should return a reference to the assigned object. That is it should be declared like

MyString & operator=(const MyString& rhs);

The string stored in the assigned object of the class MyString can be shorter than the string stored in the assigning object. In this case this statement

strcpy(this->str, rhs.str);

can result in memory overwriting beyond the allocated string that invokes undefined bejavior.

Pay attention to that there is no need to call the standard function strlen because the data member length already stores the length of a string. The operator can be defined the following way.

const MyString& operator=(const MyString& rhs)
{ 
    if ( this != &rhs ) 
    {
        if ( this->length != rhs.length )
        {
            delete[] this->str; // Why is this required?
            this->str = new char[rhs.length + 1];
            this->length = rhs.length;
        }
        strcpy( this->str, rhs.str );
    }

    return *this;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335