locked
Please review my code (copy and move) RRS feed

  • General discussion

  • I just want to be sure that I understand the copy and move semantics. Can you please review the code and see if you can find any bugs.

    // HEADER FILE
    #pragma once
    #include <string>
    
    class Data {
    public:
    	  Data(int number);					// Constructor
    	  ~Data();							// Destructor
    	  Data(const Data&);				// Copy constructor
          Data(Data&&);						// Move constructor
          Data& operator=(const Data&);		// Copy assignment operator
          Data& operator=(Data&&);			// Move assignment operator
    
    private:
    	void swap(Data& w);
    
    private:
    	int m_number;
    };
    
    class Wrapper{
    public:
    	  Wrapper(std::string name, int number);	// Constructor
    	  ~Wrapper();								// Destructor
    	  Wrapper(const Wrapper&);					// Copy constructor
          Wrapper(Wrapper&&);						// Move constructor
          Wrapper& operator=(const Wrapper&);		// Copy assignment operator
          Wrapper& operator=(Wrapper&&);			// Move assignment operator
    
    private:
    	void swap(Wrapper& w);
    	
    private:
    	std::string m_name;
    	Data* m_pData;
    };
    
    
    // CPP FILE
    #include "StdAfx.h"
    #include "Wrapper.h"
    
    Wrapper::Wrapper(std::string name, int number) : m_name(name), m_pData(new Data(number)) {  // Constructor
    }
    
    Wrapper::~Wrapper() {
    	if (m_pData) {
    		delete m_pData;
    		m_pData = nullptr;
    	}
    }
    
    Wrapper::Wrapper(const Wrapper& other) : m_name(other.m_name),
    										 m_pData(new Data(*other.m_pData))  {					// Copy constructor
    }
    
    Wrapper::Wrapper(Wrapper&& other) : m_name(std::move(other.m_name)),
    									m_pData(std::move(other.m_pData)) {						// Move constructor
    	other.m_pData = nullptr;
    }
    
    Wrapper& Wrapper::operator=(const Wrapper& other) {		// Copy assignment operator
    	Wrapper(other).swap(*this);  
        return *this;
    }
    
    Wrapper& Wrapper::operator=(Wrapper&& other) {			// Move assignment operator
    	std::swap(*this, other);
    	return *this;
    }
    
    void Wrapper::swap(Wrapper& w) throw() {	
    	std::swap(this->m_name, w.m_name);
    	std::swap(this->m_pData, w.m_pData);
    }
    
    /***********************************************************************************************
    								DATA CLASS IMPLEMENTATION.
    ***********************************************************************************************/
    
    Data::Data(int number) : m_number(number) {									// Constructor
    }
    
    Data::~Data() {
    }
    
    Data::Data(const Data& other) : m_number(other.m_number) {					// Copy constructor
    }
    
    Data::Data(Data&& other) : m_number(std::move(other.m_number)) {			// Move constructor
    }
    
    Data& Data::operator=(const Data& other) {									// Copy assignment operator
    	Data(other).swap(*this);  
        return *this;
    }
    
    Data& Data::operator=(Data&& other) {										// Move assignment operator
    	std::swap(*this, other);
    	return *this;
    }
    
    void Data::swap(Data& w) throw() {	
    	std::swap(this->m_number, w.m_number);
    }
    
    
    // MAIN.CPP
    #include "stdafx.h"
    #include "Wrapper.h"
    
    Wrapper CreateWrapper() {
    	Wrapper wrapper("My name is temporary wrapper", 3);
    	return wrapper;
    }
    
    int main(int argc, char* argv[])
    {
    
    	
    	Wrapper w1("My name is w1", 1), w2("My name is w2", 2);
    	w1 = w2;
    	Wrapper w3 = w1;
    	{
    	Wrapper w4 = CreateWrapper();
    	}
    	return 0;
    }
    
    
    
    

    • Changed type Rob Pan Monday, January 30, 2012 7:43 AM this should be a discussion
    Saturday, January 28, 2012 9:33 AM

All replies

  • I think your code is correct. but in order to check your code better, I suggest you can change the input of your project.  for example, do not define the w1,w2, or user can define w1 or w2.

    Monday, January 30, 2012 7:40 AM