C++ SQLite insert statement

Question

I insert a value into a SQLite database and return the id of the last inserted value as a result of the method.

I asked a question on Stackoverflow and I found this solution to my problem. Now I would like to get feedback if there is a simpler, smarter and/or safer way to do that:

int MyRepository::Insert(std::shared_ptr<MyObject> myObject)
{
 if(myObject == nullptr) { return -1; }
 sqlite3_exec(_connection, "BEGIN TRANSACTION;", NULL, NULL, NULL);
 std::string query = R"(
 INSERT INTO myObject (EquipmentId, ExerciseId ) VALUES (?1, ?2);
 )";
 int lastInsertedId = -1;
 sqlite3_stmt* sqlStatement;
 sqlite3_prepare_v2(_connection, query.c_str(), -1, &sqlStatement, nullptr);
 sqlite3_bind_int (sqlStatement, 1, myObject->GetEquipmentId());
 sqlite3_bind_int (sqlStatement, 2, myObject->GetExerciseId() );
 int returnCode = sqlite3_step(sqlStatement);
 if (returnCode != SQLITE_DONE)
 {
 throw SqliteException(returnCode, std::string(sqlite3_errmsg(_connection)));
 }
 query = "SELECT last_insert_rowid()";
 returnCode = sqlite3_prepare_v2(_connection, query.c_str(), -1, &sqlStatement, NULL);
 if (returnCode != SQLITE_OK)
 {
 throw SqliteException(returnCode, std::string(sqlite3_errmsg(_connection)));
 }
 returnCode = sqlite3_step(sqlStatement);
 if(returnCode == SQLITE_ROW)
 {
 lastInsertedId = sqlite3_column_int(sqlStatement, 0);
 }
 sqlite3_exec(_connection, "COMMIT;", NULL, NULL, NULL);
 sqlite3_finalize(sqlStatement);
 return lastInsertedId;
}

Answer 1

std::shared_ptr has a contextual conversion to bool, just like raw pointers. So instead of comparing to nullptr, you can simply write

if (!myObject)

The raw string seems to be unnecessary:

std::string query =
 "INSERT INTO myObject (EquipmentId, ExerciseId ) VALUES (?1, ?2)";

I don't think you need the ; statement terminator in SQL queries that are already bounded by C++ API calls - they are needed in an interactive SQL shell but not here.

When we exit early (via throw of an exception), is there some mechanism to abort the transaction for us? And to finalize the statement?

Prefer to declare your variables nearer to where they are used - specifically, lastInsertedId is a long way from where you use it:

 int lastInsertedId = -1;
 if(returnCode == SQLITE_ROW)
 {
 lastInsertedId = sqlite3_column_int(sqlStatement, 0);
 }

Or even

 const int lastInsertedId = returnCode == SQLITE_ROW
 ? sqlite3_column_int(sqlStatement, 0)
 : -1;

• Bruno Bieri: Thanks for the reply. To compare a std::shared_ptr in that way is actually a coding guideline in the project I work. The query I'll change as you suggested. The throw is handled outside of the method. I've to check about finalizing the statement. I'll change int lastInsertedId = -1; and move it to the very first point of the method and use it also in within the first if statement as the return value. Any hints about possible threading problems?
• Toby Speight: I don't know enough about the SQLite library to know what's thread-safe and what isn't, so I can't be much help there - sorry!