Не копировать массивы символов, функция подкачки оленья кожа компиляции правильно и stringPtr не изменяется

Question

// В файле заголовка: определение класса:

class myString 
{ 
public: 

     myString(void); 
     myString(const char *str); 

     myString(const myString &); //copy constructor 
     ~myString(void); //destructor 

     void swap(myString &from); 


private: 

     char *stringPtr; 
     int stringLen; 
}; 

// в CPP файле, определяя их функции-члены

myString::myString(const char *str) 
{ 
    stringLen = strlen(str); 

    stringPtr = new char[stringLen+1]; 

    strcpy(stringPtr,str); 
    cout << "constructor with parameter called"<<endl; 
} 

myString::myString(const myString &str) 
{ 

    stringPtr = new char[str.stringLen +1]; 
    strcpy(stringPtr,str.stringPtr); 
    cout << "copyconstructor"<<endl; 
} 


void myString::swap(myString &from) 
{ 
    myString buffer(from); 
    int lengthBuffer = from.stringLen; 

    from = new char[stringLen+1]; 
    from.stringLen = stringLen; 
    strcpy(from.stringPtr, stringPtr); 


    stringPtr = new char[lengthBuffer+1]; 
    stringLen = lengthBuffer; 
    strcpy(stringPtr,buffer.stringPtr); 
} 

Answer 1

Вы не можете изменить ссылку. Даже если вы замените его указателем, изменяющим указатель, он не будет изменять объект, на который указывает. Вместо этого вам нужно работать через ссылку - просто поменяйте поля.

void myString::swap(myString &from) 
{ 
    std::swap(stringLen, from.stringLen); 
    std::swap(stringPtr, from.stringPtr); 
} 

выше использует станд :: своп(), как было предложено user sbi в комментариях. Это полностью эквивалентно следующему (только для иллюстрации, не изобретайте STL):

void myString::swap(myString &from) 
    // First remember own length and pointer 
    const int myOldLen = stringLen; 
    char* myOldPtr = stringPtr; 
    // now copy the length and pointer from that other string 
    stringLen = from.stringLen; 
    stringPtr = from.stringPtr; 
    // copy remembered length and pointer to that other string 
    from.StringLen = myOldLen; 
    from.StringPtr = myOldPtr; 
    // done swapping 
} 

И будет работать даже тогда, когда называют сюда самостоятельной свопинга:

myString string; 
string.swap(string); 

Answer 2

from = new char[stringLen+1]; должен быть from.stringPtr = new char[stringLen+1];. Также не забудьте освободить ранее выделенную память до выделения нового.

Answer 3

Посмотрите внимательно на линии

from = new char[stringLen+1]; 

Это то же самое, как

from = MyString(new char[stringLen+1]); 

так что ваш конструктор MyString получить неинициализированный массив символов. Затем вы пытаетесь получить длину строки, но strlen просто перебираете символы строки, ищущей 0 char. Поскольку мы не знаем, какой контент может содержать неинициализированный массив символов, мы не знаем, какую длину может вернуть strlen. Он может даже идти дальше границы массива и разбивать вашу программу на segfault. Но я могу сказать точно, после этого недостаточно места в from.stringPtr, чтобы удерживать строку, которую вы хотите скопировать.

Итак, используйте from.stringPtr = new char[stringLen+1]; или лучше from = MyString(*this);, так как у вас уже есть экземпляр-конструктор.

Answer 4

Вы уже получили несколько хороших ответов, касающихся ошибок в вашей функции myString::swap(). Тем не менее, я хотел бы добавить еще один. В этой функции есть кое-что неправильное, мне сначала было трудно думать о том, с чего начать. Но потом я понял, что вы не на какой-то фундаментальный вопрос, который я хотел бы отметить:

В конвенции, функция называется swap, как ожидается, выполнять свою задачу

1. в O (1)
2. не бросая исключение.

(Да, я знаю, есть исключения: std::tr1::array<>::swap(). Но это должно быть очень обоснованным.) Ваша реализация не работает на обеих учетных записях. Это O (n) (strcpy) и может вызвать исключение (new) - и делает это без необходимости и без каких-либо оснований.

Если вы посмотрите на myString, вы увидите, что у него есть только два элемента данных элемента, оба из которых имеют встроенный тип. Это означает, что обмен двумя объектами этого класса действительно прост в использовании, соблюдая приведенные выше соглашения: просто поменяйте данные элемента. Это так же просто, как вызов std::swap на них:

void myString::swap(myString &from) 
{ 
    std::swap(this->stringPtr,from.stringPtr); 
    std::swap(this->stringLen,from.stringLen); 
} 

Это никогда не подведет (поменяв два указателя и два целых числа, не может не), выполняется в O (1), очень легко понять (ну, как только вы получите так или иначе, это идиоматическая форма реализации функции swap, специфичной для класса) и состоит из двух строк кода, вызывающих что-то хорошо протестированное в стандартной библиотеке вместо 8 строк кода, делающих ошибки (например, и, в вашем случае, ошибочное) ручное управление памятью.

Примечание 1: После того как вы сделали это, вы должны специализироваться std::swap назвать вашу реализацию для своего класса:

namespace std { // only allowed for specializing function templates in the std lib 
    template<> 
    inline void std::swap<myString>(myString& lhs, myString& rhs) 
    { 
    lhs.swap(rhs); 
    } 

Примечания 2: Лучшее (простого, исключение безопасного, и сам-assignment- безопасный) способ реализации назначения для своего класса, чтобы использовать его swap:

myString& myString::operator=(const myString& rhs) 
{ 
    myString tmp(rhs); // invoke copy ctor 
    this->swap(tmp); // steal data from temp and leave it with our own old data 
    return *this; 
} // tmp will automatically be destroyed and takes our old data with it