Small improvements & code cleanup

Author: Kacperos155

include/SQLiteCpp/Backup.h

@@ -14,6 +14,7 @@
 #include <SQLiteCpp/Database.h>
 
 #include <string>
+#include <memory>
 
 // Forward declaration to avoid inclusion of <sqlite3.h> in a header
 struct sqlite3_backup;
@@ -95,9 +96,6 @@ class Backup
     Backup(const Backup&) = delete;
     Backup& operator=(const Backup&) = delete;
 
-    /// Release the SQLite Backup resource.
-    ~Backup();
-
     /**
      * @brief Execute a step of backup with a given number of source pages to be copied
      *
@@ -114,14 +112,19 @@ class Backup
     int executeStep(const int aNumPage = -1);
 
     /// Return the number of source pages still to be backed up as of the most recent call to executeStep().
-    int getRemainingPageCount();
+    int getRemainingPageCount() const;
 
     /// Return the total number of pages in the source database as of the most recent call to executeStep().
-    int getTotalPageCount();
+    int getTotalPageCount() const;
 
 private:
-    // TODO: use std::unique_ptr with a custom deleter to call sqlite3_backup_finish()
-    sqlite3_backup* mpSQLiteBackup = nullptr;   ///< Pointer to SQLite Database Backup Handle
+    // Deleter functor to use with smart pointers to close the SQLite database backup in an RAII fashion.
+    struct Deleter
+    {
+        void operator()(sqlite3_backup* apBackup);
+    };
+
+    std::unique_ptr<sqlite3_backup, Deleter> mpSQLiteBackup{};   ///< Pointer to SQLite Database Backup Handle
 };
 
 }  // namespace SQLite

include/SQLiteCpp/Database.h

@@ -78,8 +78,8 @@ extern const int OPEN_NOFOLLOW;     // SQLITE_OPEN_NOFOLLOW
 
 extern const int OK;                ///< SQLITE_OK (used by check() bellow)
 
-extern const char*  VERSION;        ///< SQLITE_VERSION string from the sqlite3.h used at compile time
-extern const int    VERSION_NUMBER; ///< SQLITE_VERSION_NUMBER from the sqlite3.h used at compile time
+extern const char* const VERSION;        ///< SQLITE_VERSION string from the sqlite3.h used at compile time
+extern const int         VERSION_NUMBER; ///< SQLITE_VERSION_NUMBER from the sqlite3.h used at compile time
 
 /// Return SQLite version string using runtime call to the compiled library
 const char* getLibVersion() noexcept;
@@ -328,7 +328,7 @@ class Database
      *
      * @return the sqlite result code.
      */
-    int tryExec(const std::string aQueries) noexcept
+    int tryExec(const std::string& aQueries) noexcept
     {
         return tryExec(aQueries.c_str());
     }
@@ -389,7 +389,7 @@ class Database
      *
      * @throw SQLite::Exception in case of error
      */
-    bool tableExists(const char* apTableName);
+    bool tableExists(const char* apTableName) const;
 
     /**
      * @brief Shortcut to test if a table exists.
@@ -402,7 +402,7 @@ class Database
      *
      * @throw SQLite::Exception in case of error
      */
-    bool tableExists(const std::string& aTableName)
+    bool tableExists(const std::string& aTableName) const
     {
         return tableExists(aTableName.c_str());
     }
@@ -552,7 +552,7 @@ class Database
     static Header getHeaderInfo(const std::string& aFilename);
 
     // Parse SQLite header data from a database file.
-    Header getHeaderInfo()
+    Header getHeaderInfo() const
     {
         return getHeaderInfo(mFilename);
     }

include/SQLiteCpp/Savepoint.h

@@ -66,7 +66,7 @@ class Savepoint {
      * Exception is thrown in case of error, then the Savepoint is NOT
      * initiated.
      */
-    Savepoint(Database& aDatabase, std::string name);
+    Savepoint(Database& aDatabase, const std::string& name);
 
     // Savepoint is non-copyable
     Savepoint(const Savepoint&) = delete;
@@ -88,8 +88,8 @@ class Savepoint {
     void rollback();
 
    private:
-    Database& mDatabase;  ///< Reference to the SQLite Database Connection
-    std::string msName;   ///< Name of the Savepoint
-    bool mbReleased;      ///< True when release has been called
+    Database& mDatabase;        ///< Reference to the SQLite Database Connection
+    std::string msName;         ///< Name of the Savepoint
+    bool mbReleased{ false };   ///< True when release has been called
 };
 }  // namespace SQLite

include/SQLiteCpp/Statement.h

@@ -74,18 +74,14 @@ class Statement
         Statement(aDatabase, aQuery.c_str())
     {}
 
-    /**
-     * @brief Move an SQLite statement.
-     *
-     * @param[in] aStatement    Statement to move
-     */
-    Statement(Statement&& aStatement) noexcept;
-    Statement& operator=(Statement&& aStatement) noexcept = default;
-
     // Statement is non-copyable
     Statement(const Statement&) = delete;
     Statement& operator=(const Statement&) = delete;
 
+    Statement(Statement&& aStatement) noexcept;
+    Statement& operator=(Statement&& aStatement) noexcept = default;
+    // TODO: Change Statement move constructor to default
+
     /// Finalize and unregister the SQL query from the SQLite Database Connection.
     /// The finalization will be done by the destructor of the last shared pointer
     ~Statement() = default;
@@ -705,7 +701,7 @@ class Statement
     int                     mColumnCount{0};        //!< Number of columns in the result of the prepared statement
     bool                    mbHasRow{false};        //!< true when a row has been fetched with executeStep()
     bool                    mbDone{false};          //!< true when the last executeStep() had no more row to fetch
-    
+
     /// Map of columns index by name (mutable so getColumnIndex can be const)
     mutable std::map<std::string, int>  mColumnNames{};
 };

include/SQLiteCpp/Transaction.h

@@ -87,8 +87,8 @@ class Transaction
     void commit();
 
 private:
-    Database&   mDatabase;  ///< Reference to the SQLite Database Connection
-    bool        mbCommited; ///< True when commit has been called
+    Database&   mDatabase;              ///< Reference to the SQLite Database Connection
+    bool        mbCommited{ false };    ///< True when commit has been called
 };
 
 

src/Backup.cpp

@@ -24,10 +24,10 @@ Backup::Backup(Database&    aDestDatabase,
                Database&    aSrcDatabase,
                const char*  apSrcDatabaseName)
 {
-    mpSQLiteBackup = sqlite3_backup_init(aDestDatabase.getHandle(),
+    mpSQLiteBackup.reset(sqlite3_backup_init(aDestDatabase.getHandle(),
                                          apDestDatabaseName,
                                          aSrcDatabase.getHandle(),
-                                         apSrcDatabaseName);
+                                         apSrcDatabaseName));
     if (nullptr == mpSQLiteBackup)
     {
         // If an error occurs, the error code and message are attached to the destination database connection.
@@ -48,19 +48,10 @@ Backup::Backup(Database &aDestDatabase, Database &aSrcDatabase) :
 {
 }
 
-// Release resource for SQLite database backup
-Backup::~Backup()
-{
-    if (mpSQLiteBackup)
-    {
-        sqlite3_backup_finish(mpSQLiteBackup);
-    }
-}
-
 // Execute backup step with a given number of source pages to be copied
 int Backup::executeStep(const int aNumPage /* = -1 */)
 {
-    const int res = sqlite3_backup_step(mpSQLiteBackup, aNumPage);
+    const int res = sqlite3_backup_step(mpSQLiteBackup.get(), aNumPage);
     if (SQLITE_OK != res && SQLITE_DONE != res && SQLITE_BUSY != res && SQLITE_LOCKED != res)
     {
         throw SQLite::Exception(sqlite3_errstr(res), res);
@@ -69,15 +60,24 @@ int Backup::executeStep(const int aNumPage /* = -1 */)
 }
 
 // Get the number of remaining source pages to be copied in this backup process
-int Backup::getRemainingPageCount()
+int Backup::getRemainingPageCount() const
 {
-    return sqlite3_backup_remaining(mpSQLiteBackup);
+    return sqlite3_backup_remaining(mpSQLiteBackup.get());
 }
 
 // Get the number of total source pages to be copied in this backup process
-int Backup::getTotalPageCount()
+int Backup::getTotalPageCount() const
 {
-    return sqlite3_backup_pagecount(mpSQLiteBackup);
+    return sqlite3_backup_pagecount(mpSQLiteBackup.get());
+}
+
+// Release resource for SQLite database backup
+void SQLite::Backup::Deleter::operator()(sqlite3_backup* apBackup)
+{
+    if (apBackup)
+    {
+        sqlite3_backup_finish(apBackup);
+    }
 }
 
 }  // namespace SQLite

src/Column.cpp

@@ -78,7 +78,7 @@ double Column::getDouble() const noexcept
 const char* Column::getText(const char* apDefaultValue /* = "" */) const noexcept
 {
     auto pText = reinterpret_cast<const char*>(sqlite3_column_text(mStmtPtr.get(), mIndex));
-    return (pText?pText:apDefaultValue);
+    return (pText ? pText : apDefaultValue);
 }
 
 // Return a pointer to the blob value (*not* NULL terminated) of the column specified by its index starting at 0

src/Database.cpp

@@ -27,6 +27,7 @@
 namespace SQLite
 {
 
+const int   OK                = SQLITE_OK;
 const int   OPEN_READONLY     = SQLITE_OPEN_READONLY;
 const int   OPEN_READWRITE    = SQLITE_OPEN_READWRITE;
 const int   OPEN_CREATE       = SQLITE_OPEN_CREATE;
@@ -42,10 +43,8 @@ const int   OPEN_NOFOLLOW     = SQLITE_OPEN_NOFOLLOW;
 const int   OPEN_NOFOLLOW     = 0;
 #endif
 
-const int   OK              = SQLITE_OK;
-
-const char* VERSION         = SQLITE_VERSION;
-const int   VERSION_NUMBER  = SQLITE_VERSION_NUMBER;
+const char* const VERSION        = SQLITE_VERSION;
+const int         VERSION_NUMBER = SQLITE_VERSION_NUMBER;
 
 // Return SQLite version string using runtime call to the compiled library
 const char* getLibVersion() noexcept
@@ -142,7 +141,7 @@ Column Database::execAndGet(const char* apQuery)
 }
 
 // Shortcut to test if a table exists.
-bool Database::tableExists(const char* apTableName)
+bool Database::tableExists(const char* apTableName) const
 {
     Statement query(*this, "SELECT count(*) FROM sqlite_master WHERE type='table' AND name=?");
     query.bind(1, apTableName);
@@ -439,8 +438,8 @@ void Database::backup(const char* apFilename, BackupType aType)
     Database otherDatabase(apFilename, SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE);
 
     // For a 'Save' operation, data is copied from the current Database to the other. A 'Load' is the reverse.
-    Database& src = (aType == Save ? *this : otherDatabase);
-    Database& dest = (aType == Save ? otherDatabase : *this);
+    Database& src = (aType == BackupType::Save ? *this : otherDatabase);
+    Database& dest = (aType == BackupType::Save ? otherDatabase : *this);
 
     // Set up the backup procedure to copy between the "main" databases of each connection
     Backup bkp(dest, src);

src/Savepoint.cpp

@@ -18,8 +18,8 @@
 namespace SQLite {
 
 // Begins the SQLite savepoint
-Savepoint::Savepoint(Database& aDatabase, std::string aName)
-    : mDatabase(aDatabase), msName(aName), mbReleased(false) {
+Savepoint::Savepoint(Database& aDatabase, const std::string& aName)
+    : mDatabase(aDatabase), msName(aName) {
     // workaround because you cannot bind to SAVEPOINT
     // escape name for use in query
     Statement stmt(mDatabase, "SELECT quote(?)");

src/Transaction.cpp

@@ -21,8 +21,7 @@ namespace SQLite
 
 // Begins the SQLite transaction
 Transaction::Transaction(Database& aDatabase, TransactionBehavior behavior) :
-    mDatabase(aDatabase),
-    mbCommited(false)
+    mDatabase(aDatabase)
 {
     const char *stmt;
     switch (behavior) {
@@ -43,8 +42,7 @@ Transaction::Transaction(Database& aDatabase, TransactionBehavior behavior) :
 
 // Begins the SQLite transaction
 Transaction::Transaction(Database &aDatabase) :
-    mDatabase(aDatabase),
-    mbCommited(false)
+    mDatabase(aDatabase)
 {
     mDatabase.exec("BEGIN");
 }