0

I tried to use unique_ptr in c++ in a singelton pattern instead of raw pointer. when I want to assign a unique_ptr to another I got an error. I tried to use std::move to assign but it did not work. the code is as follow:

#include <iostream>
#include <memory>
#include <list>
#include <algorithm>
#include <iterator>
#include <string>

using namespace std;

class ClientDB
{
  private:
    static unique_ptr<ClientDB> theDB;
    ClientDB() {}
    list<string> clients;
  public:
    ~ClientDB() {}
    static unique_ptr<ClientDB> getInstance()
    {
      if(theDB==nullptr)
    theDB = make_unique<ClientDB>;
      return theDB;
    }
    void addClient(string c) {clients.push_back(c);}
    void printClients(ostream& os)
    {
      copy(clients.cbegin(),clients.cend(),ostream_iterator<string>{os,"\n"});
    }
};


int main()
{
  unique_ptr<ClientDB> db1{ClientDB::getInstance()};
  db1->addClient("Mr. Schultz");
  unique_ptr<ClientDB> db2{ClientDB::getInstance()};
  db2->addClient("Mrs. James");
  unique_ptr<ClientDB> db3{ClientDB::getInstance()};
  db3->addClient("Mr. Karajan");
  db1->printClients(cout);
}

and the error I got is

error: no match for ‘operator=’ (operand types are ‘std::unique_ptr<ClientDB>’ and ‘<unresolved overloaded function type>’)
     theDB = make_unique<ClientDB>;

and another question is if nullptr can be used for unique_ptr.

  • 3
    `make_unique` needs to be **called**. `make_unique()`. Mind you, you approach is still broken. You can't return a unique_ptr while holding on to the ownership of the object. – StoryTeller - Unslander Monica Jan 27 '19 at 07:19
  • @StoryTeller you're right. this is another error I got `error: use of deleted function ‘std::unique_ptr<_tp _dp="">::unique_ptr(const std::unique_ptr<_tp _dp="">&) [with _Tp = ClientDB; _Dp = std::default_delete]’ return theDB;' –  Jan 27 '19 at 07:25
  • 1
    Yeah, that's the copy constructor (which doesn't exist because `unique_ptr` wants to be ... well, unique). – melpomene Jan 27 '19 at 07:30
  • Basically, `return theDB` is trying to create a copy of a `unique_ptr`, which is forbidden. You may return a reference `unique_ptr&` or the raw pointer managed by the `unique_ptr`: `return theDB.get()`. – ChronoTrigger Jan 27 '19 at 08:04
  • Don't use Singleton, and if you do, don't use unique_ptr for it. See https://stackoverflow.com/a/1008289/2466431 – JVApen Jan 27 '19 at 11:47

1 Answers1

0

Finally by help of my teacher, I can solve the problem. there is some points should be considered.

1- for unique_ptr assignment, std::move:: should be used.

2- make_unique has no access to the private constructor, the constructor should be called explicitly: theDB = unique_ptr<ClientDB>(new ClientDB());

3-The unique-ptr must be initialized outside the class: unique_ptr<ClientDB> ClientDB::theDB;

4- Three unique pointers in main for the same object can not be used, only one is allowed for uniqueness. references to unique_ptr should be used instead: unique_ptr<ClientDB>& db1=ClientDB::getInstance();

and finally the code should be like this

#include <iostream>
#include <memory>
#include <list>
#include <algorithm>
#include <iterator>
#include <string>

using namespace std;

class ClientDB
{
private:
  static unique_ptr<ClientDB> theDB;
  ClientDB() {}
  list<string> clients;
public:
  ~ClientDB() {}
  static unique_ptr<ClientDB>& getInstance()
  {
    if(theDB==nullptr)
      //theDB = move(make_unique<ClientDB>());
      theDB = unique_ptr<ClientDB>(new ClientDB());
    return theDB;
  }
  void addClient(string c) {clients.push_back(c);}
  void printClients(ostream& os)
  {
    copy(clients.cbegin(),clients.cend(),ostream_iterator<string>{os,"\n"});
  }
};
unique_ptr<ClientDB> ClientDB::theDB;

int main()
{
  unique_ptr<ClientDB>& db1=ClientDB::getInstance();
  db1->addClient("Mr. Schultz");


  unique_ptr<ClientDB>& db2=ClientDB::getInstance();
  db2->addClient("Mrs. James");


  unique_ptr<ClientDB>& db3=ClientDB::getInstance();
  db3->addClient("Mr. Karajan");

  db1->printClients(cout);
}
  • A reference to `unique_ptr` is usually pointless. The `getInstance` should return either a raw pointer `ClientDB*` or a reference `ClientDB&`, since the caller is not involved at all in managing the lifetime of the object. Unless you want code to be able to steal the object by `unique_ptr mine = std::move(ClientDB::getInstance());` and make it not really a singleton! – aschepler Feb 01 '19 at 12:10