Cloning vector of pointers

Question

Hello I'm new here and new to C++. I have a problem where I need to make backup copy of my vector of pointers. But I can't relly get it properly. I found solution to my case on this forum but can't relly get it right:

class cloneFunctor {
public:
    T* operator() (T* a) {
        return a->clone();
    }
}

I tried to implement this into my code but could't get a good resolve, could anyone help me to get this thing right?

My code:

sever.cpp

  #include "server.h"
    #include <iostream>
    #include <functional>
    #include <algorithm>
    #include <iterator>
    class Client;


    class cloneFunctor {
    public:
        cloneFunctor* operator() (cloneFunctor* a) {
            return a->clone();
        }
    };

    Server *Server::instance = 0;

    Server& Server::getInstance() {
        if (instance == 0)
            instance = new Server();
        return (*instance);
    }

    void Server::setStatus(bool status) {
        this->nStatus = status;
        changeClientStatus();
        writeStateToConsole();
    }

    bool Server::getStatus() {
        return nStatus;
    }

    void Server::writeStateToConsole() {
        std::cout << "Server state: " << getStatus() << std::endl;
    }

    void Server::subscribeToServer(Client &temp) {
        listOfClients.push_back(&temp);
    }
    void Server::writeClients() {
        for (unsigned int i = 0; i < listOfClients.size(); i++) {
            std::cout << i+1  << ". client status: " << listOfClients[i]->getStatus() << std::endl;
        }
    }

    void Server::changeClientStatus() {
        if (nStatus == 0){
            makeCopy(listOfClients);
            for (unsigned int i = 0; i < listOfClients.size(); i++) {
                listOfClients[i]->setStatus(false);
            }
        }
        else
            restoreCopy();

    }

    void Server::makeCopy(std::vector<Client *>listOfClients) {
         transform(listOfClients.begin(), listOfClients.end(), back_inserter(listOfClientsOld), cloneFunctor());
    }
    void Server::restoreCopy() {


    }

server.h

#ifndef SERVER_H_
#define SERVER_H_

#include "abstractbaseclass.h"
#include "server.h"
#include "client.h"



class Client;
class Server : public Abstractbaseclass {
    friend class Client;
public:
    static Server& getInstance();
    virtual void setStatus(bool status);
    virtual bool getStatus();
    virtual void writeStateToConsole();
    void subscribeToServer(Client &temp);
    void writeClients();
    void changeClientStatus();
    void makeCopy(std::vector<Client *>listOfClients);
    void restoreCopy();


 private:
    static Server *instance;
    Server(){};
    std::vector <Client *>listOfClients;
    std:: vector <Client *> listOfClientsOld;
};


#endif /* SERVER_H_ */

Program should create singleton Server class, and then create 3 clients who will subsribe to server (these are hold in vector of pointers). When I set server status to 0, all clients change their state to off (bool false) and befeore this should be created backup vector, becouse when I'll turn on server again clients need to switch to their state from before shutting down server.

Answer 1

OK, so he's trying to teach some sort of transaction-based thinking.

Client may need an assignment operator

Client & operator=(Client & toCopy)
{
    // copy all of toCopy's members over to this
}

if it doesn't already have one and contains pointers or complex data types that you can't trust to self-copy.

Now you can easily

clientA = clientB;

to copy clientB into clientA

Then we get to the main event, Server::restoreCopy(), and it's a brutally simple variant of:

if (listOfClients.size() != listOfClientsOld.size())
{
    // back up is stale. Probably throw exception
}
for (size_t index = 0; index < listOfClients.size(); index++)
{
    *listOfClients[index] = *listOfClientsOld[index];
}

These are vectors of pointers so you must dereference (*) the pointer to get the pointed at Client or you copy the pointer and not the pointed at. Remember that clone of Client is a totally different Clientand not what Server's user passed in in the first place. If the Server user is still holding onto the initial Client and using it, bad ju-ju will happen if you only copy the pointers. Both sides will be operating on different Clients even though they may look the same. So much as one modification and they won't

You can also use the std::transform trick used to make the back-up here as well, but this is simple and obvious and easy to debug. Feel free to use iterators and range-based for as well.

And while you are at it, pray no one has reordered listOfClients since you made the back-up.

Important side notes:

void makeCopy(std::vector<Client *>listOfClients);

is weird. It allows a Server user to pass in their own vector and add to any existing back-ups. This includes Server itself. It does not remove any existing back up, so listOfClientsOld will keep growing. And because anyone who has access to the Server can call it and pass in their own vector, they can stuff that back up full of fallacious nonsense. I'd recommend that makeCopy take no parameters and remove any existing Clients from the backup, and delete them. Better still, listOfClientsOld has no need to store pointers at all and probably shouldn't.

Lots of pointers in play and no deletes. Somebody has to give back all of that memory you're allocating or you'll eventually run out.