Не копирует массивы char, функция swap не компилируется правильно, а 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 );
}

вышеприведенное использует std :: swap (), как предложено пользователем 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

Вы уже получили несколько хороших ответов об ошибках в вашей функции 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: Лучший (простой, безопасный для исключений и безопасный для самостоятельного присваивания) способ реализовать присвоение для вашего класса - это использовать его 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

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

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