Question

Here is my scenario:

class Database {

    public:
        Database();
        ~Database();

        void close();
                ...

    private:

        sqlite3 *database; //SQLITE3 OBJECT
        bool isOpenDb;
        ...

};

Database::Database() {

    database = 0;
    filename = "";
    isOpenDb = false;
}

Database::~Database() {
    close();
}

void Database::close() {
    sqlite3_close(database); 
    isOpenDb = false;
}

When Database object is destroyed, I want that the sqlite3 object is closed. In this form, it seems that it can be an unsafe approach since if I copy the object and I destroy it, then the copied object is in an invalid state.

In your opinion, what is the best way to proceed? I was thinking about singleton class but I'm not sure about this.

Was it helpful?

Solution

Make your class noncopyable. In C++11:

class Database {
public:
  Database(const Database&) = delete;
  Database& operator=(const Database&) = delete;
};

In C++03:

class Database {
private: // inaccessible 
  Database(const Database&);
  Database& operator=(const Database&);
};

Or with Boost:

#include <boost/noncopyable.hpp>
class Database : private boost::noncopyable {
};

In C++11 I would also make the object Movable by giving it a MoveConstructor and Move assignment operators. There you would assign the handle (or whatever your db gives you) to the new object and use some flag in the old object to indicate that nothing needs to be closed.

Don't forget to implement swap as well!

class Database {
  // This might look odd, but is the standard way to do it without a namespace.
  // If you have a namespace surrounding Database, put it there.
  friend void swap(Database& a, Database& b) { /* code to swap a and b */ }
};

Also: setting some value to false in your destructor has no effect. Nothing should ever be able to see the change.

Or using a unique_ptr/shared_ptr with a custom deleter:

struct CloseDatabase {
  void operator()(sqlite* x) { sqlite3_close(x); }
};

typedef std::unique_ptr<sqlite3, CloseDatabase> Database;

OTHER TIPS

You could use a shared pointer [0], considering your application is single-threaded.

The copy-constructor will then be trivial, and the object will be destroyed when the last instantiation of your class is destroyed.

private:
  std::shared_ptr<sqlite3> database;

EDIT: As pointed out by pmr, a custom deleter is needed in this case to close the database. For example a simple lambda :

database = std::shared_ptr<sqlite3>(new database,
                                    [](sqlite3 *db)
                                    { db->close; delete db; });

Or you can use a functor as a deleter.

Singleton will make your class non-copyable, if that's what you want, you can do this.

Or just disable copy constructor and copy operator (and move equivalents). C++11 provides the keyword delete to do this.

Database(const Database &) = delete;
Database & operator =(const Database &) = delete;
Database(Database &&) = delete;
Database & operator =(Database &&) = delete;

[0] http://en.cppreference.com/w/cpp/memory/shared_ptr

Unless there's a good reason to do otherwise, disable the copy constructor and assignment operator and do not permit copies. If there's a good reason to permit copies, then exploring what that reason is will help you understand what semantics you actually want when there is a copy and that should lead to a design.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top