Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small improvements and code cleaning #360

Merged
merged 9 commits into from
Sep 18, 2022
17 changes: 10 additions & 7 deletions include/SQLiteCpp/Backup.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*
Expand All @@ -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()
SRombauts marked this conversation as resolved.
Show resolved Hide resolved
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
Kacperos155 marked this conversation as resolved.
Show resolved Hide resolved
{
void operator()(sqlite3_backup* apBackup);
};

std::unique_ptr<sqlite3_backup, Deleter> mpSQLiteBackup{}; ///< Pointer to SQLite Database Backup Handle
Kacperos155 marked this conversation as resolved.
Show resolved Hide resolved
};

} // namespace SQLite
12 changes: 6 additions & 6 deletions include/SQLiteCpp/Database.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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.
Expand All @@ -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());
}
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions include/SQLiteCpp/Savepoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Kacperos155 marked this conversation as resolved.
Show resolved Hide resolved
};
} // namespace SQLite
14 changes: 5 additions & 9 deletions include/SQLiteCpp/Statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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{};
};
Expand Down
4 changes: 2 additions & 2 deletions include/SQLiteCpp/Transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Kacperos155 marked this conversation as resolved.
Show resolved Hide resolved
};


Expand Down
32 changes: 16 additions & 16 deletions src/Backup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -48,19 +48,10 @@ Backup::Backup(Database &aDestDatabase, Database &aSrcDatabase) :
{
}

// Release resource for SQLite database backup
Kacperos155 marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand All @@ -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
2 changes: 1 addition & 1 deletion src/Column.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 6 additions & 7 deletions src/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/Savepoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(?)");
Expand Down
6 changes: 2 additions & 4 deletions src/Transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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");
}
Expand Down