2011-01-10 2 views
3

Рассмотрим следующий код:Понимание C++ динамическое распределение

class CString 
{ 
private: 
    char* buff; 
    size_t len; 

public: 
    CString(const char* p):len(0), buff(nullptr) 
    { 
     cout << "Constructor called!"<<endl; 
     if (p!=nullptr) 
     { 
      len= strlen(p); 
      if (len>0) 
      { 
       buff= new char[len+1]; 
       strcpy_s(buff, len+1, p);    
      }   
     }  
    } 

    CString (const CString& s) 
    { 
     cout << "Copy constructor called!"<<endl; 
     len= s.len; 
     buff= new char[len+1]; 
     strcpy_s(buff, len+1, s.buff);  
    } 

    CString& operator = (const CString& rhs) 
    { 
     cout << "Assignment operator called!"<<endl; 
     if (this != &rhs) 
     { 
      len= rhs.len; 
      delete[] buff;   
      buff= new char[len+1]; 
      strcpy_s(buff, len+1, rhs.buff); 
     } 

     return *this; 
    } 

    CString operator + (const CString& rhs) const 
    { 
     cout << "Addition operator called!"<<endl; 

     size_t lenght= len+rhs.len+1; 
     char* tmp = new char[lenght]; 
     strcpy_s(tmp, lenght, buff); 
     strcat_s(tmp, lenght, rhs.buff); 

     return CString(tmp); 
    } 

    ~CString() 
    { 
     cout << "Destructor called!"<<endl; 
     delete[] buff; 
    }  
}; 

int main() 
{ 
CString s1("Hello"); 
CString s2("World"); 
CString s3 = s1+s2;  
} 

Моя проблема заключается в том, что я не знаю, как удалить память, выделенную в функции оператора сложения (char* tmp = new char[length]). Я не мог сделать это в конструкторе (я пробовал delete[] p), потому что он также вызывается из основной функции с массивами символов в качестве параметров, которые не выделяются в куче ... Как я могу обойти это?

+0

Вы также должны работать над оператором присваивания (его небезопасно (предпочитают Copy and Swap Idium)). Смотри ниже. –

ответ

4

Функция добавления должна возвращать CString, а не CString &. В функции добавления вы должны построить возвращаемое значение, а затем удалить [] temp, поскольку он больше не нужен, поскольку внутри класса CString вы создаете копию памяти.

CString operator + (const CString& rhs) const 
{ 
    cout << "Addition operator called!"<<endl; 

    size_t lenght= len+rhs.len+1; 
    char* tmp = new char[lenght]; 
    strcpy_s(tmp, lenght, buff); 
    strcat_s(tmp, lenght, rhs.buff); 

    CString retval(tmp); 
    delete[] tmp; 
    return retval; 
} 
+0

Итак, вы создаете новый экземпляр CString внутри нестатического метода? Я не думаю, что это очень хорошая реализация вообще – trojanfoe

+2

@trojanfoe: так обычно выполняется оператор +. Как еще вы собираетесь вернуть CString, за исключением создания нового? – Puppy

+0

@DeadMG большое спасибо! это было просто, все, что я должен сделать, это создать объект, не возвращая его немедленно, чтобы успеть удалить массив! – kiokko89

0
CString& operator + (const CString& rhs) const 

{ 
    cout << "Addition operator called!"<<endl; 

    size_t lenght= len+rhs.len+1; 
    char* tmp = new char[lenght]; 
    strcpy_s(tmp, lenght, buff); 
    strcat_s(tmp, lenght, rhs.buff); 
    CString tempObj(tmp); 
    delete [] tmp; 
    return tempObj; 
} 

Например,

+1

Возвращая ссылку на локальную переменную ??? –

+0

Это была моя ошибка ... я ошибся в обратном типе, это должен быть CString, а не CString & (теперь я редактировал сообщение). В любом случае, все вы мне очень помогли ... Спасибо! – kiokko89

0

первую очередь, operator+ должен возвращать новый объект, а не изменять один из операндов +, так что лучше должен быть объявлен как не член (вероятно, друг). сначала реализуем operator+=, а затем используя его - operator+, и у вас не будет этой проблемы.

CString operator+(CString const& lh, CString const& rh) 
{ 
    CString res(lh); 
    return res += rh; 
} 
+1

Небольшая оптимизация: вы можете передать значение lh по значению. Это даст вам копию без фактического создания временного. (Я знаю, что читать становится сложнее (и я всегда добавляю комментарий о передаче по значению, когда я это делаю)) Но это также помогает компилятору с NRVO. Я считаю, что GMAN написал замечательную статью по этому вопросу. @Gman, я не могу найти его, если вы могли бы указать на это. –

+0

tnx, мне нравится этот подход –

2

Проблемы:

В вашем операторе присваивания вы не в состоянии обеспечить любые исключения гарантии. Вы удаляете буфер, прежде чем будете уверены, что операция будет успешной. Если что-то пойдет не так, ваш объект останется в неопределенном состоянии.

CString& operator = (const CString& rhs) 
{ 
    cout << "Assignment operator called!"<<endl; 
    if (this != &rhs) 
    { 
     len= rhs.len; 
     delete[] buff;   
     buff= new char[len+1]; /// BOOM 

     // If you throw here buff now points at undefined memory. 
     // If this is an automatic variable the destructor is still going 
     // to be called and you will get a double delete. 

     // All operations that can fail should be done BEFORE the object is modified. 

     strcpy_s(buff, len+1, rhs.buff); 
    } 

    return *this; 
} 

Мы можем исправить эти проблемы, перемещая вещи (и используя временную шкалу).

CString& operator = (const CString& rhs) 
{ 
    cout << "Assignment operator called!"<<endl; 
    if (this != &rhs) 
    { 
     char* tmp = new char[len+1]; 
     strcpy_s(tmp, rhs.len+1, rhs.buff); // for char this will never fail 
              // But if it was another type the copy 
              // may potentially fail. So you must 
              // do the copy before changing the curren 
              // objects state. 

     // Now we can change the state of the object safely. 
     len= rhs.len; 
     std::swap(tmp,buff); 

     delete tmp; 
    } 

    return *this; 
} 

Даже лучшим решением является использование копирования и подкачки idium:

CString& operator = (CString rhs) // Note pass by value to get auto copy. 
{         // Most compilers will then do NRVO 
    this->swap(rhs); 
    // Simply swap the tmp rhs with this. 
    // Note that tmp was created with copy constructor. 
    // When rhs goes out of scope it will delete the object. 
} 

void swap(CString& rhs) 
{ 
    std::swap(len, rhs.len); 
    std::swap(buff, rhs.buff); 
} 

Теперь позволяет сделку с оператора +

CString operator + (const CString& rhs) const 
{ 
    // You could optimize this by providing a private constructor 
    // that takes two char pointers so that allocation is only done 
    // once. 
    CString result(*this); 
    return result += rhs; 
} 

CString operator += (const CString& rhs) 
{ 
    size_t lenght= len+rhs.len+1; 

    // Char are easy. No chance of failure. 
    // But if this was a type with a copy constructor or any other complex 
    // processing involved in the copy then I would make tmp a smart pointer 
    // to make sure that it's memory was not leaked if there was an exception. 
    char* tmp = new char[lenght]; 

    strcpy_s(tmp, lenght, buff); 
    strcat_s(tmp, lenght, rhs.buff); 

    std::swap(len, length); 
    std::swap(buff, tmp); 

    delete tmp; 
} 
+0

Большое спасибо за ваше объяснение. Я просто новичок, но вы дали мне несколько полезных советов, чтобы понять, как это должно быть сделано ... – kiokko89

Смежные вопросы