NFD » ndn-cxx

I missed this before. Why "Sqlite3" appears upfront? Name should be just PibSqlite3, there is no need for additional "Impl" suffix, as Sqlite3 is implementation of Pib.

Alex Afanasyev wrote:

I missed this before. Why "Sqlite3" appears upfront? Name should be just PibSqlite3, there is no need for additional "Impl" suffix, as Sqlite3 is implementation of Pib.

But it inherits from PibImpl? And there is another class Pib, I do not want to give people an impression that PibSqlite3 inherits from Pib...

It doesn't really matter from what it inherits. "Sqlite3" suffix (for me) already indicates that it is a concrete implementation and the name does not necessarily need to reflect the inheritance.

how about PibImplSqlite3?

In commit:216a9229c43ec62fdeab3bd673a9793bc4ca66af, the need for sqlite3_finalize makes code cluttered, because that function must be called in every code path before a method returns.

I suggest introducing a helper to solve this problem:

/** \brief wraps an SQLite prepared statement
 */
class PreparedStatement
{
public:
  /** \brief prepares a statement
   *  \throw std::domain_error SQL statement is bad
   */
  PreparedStatement(sqlite3* database, const std::string& sql)
  {
    int res = sqlite3_prepare_v2(database, sql.c_str(), -1, &m_stmt, nullptr);
    if (res != SQLITE_OK) {
      throw std::domain_error("bad SQL statement: " + str);
    }
  }

  /** \brief finalizes the statement
   */
  ~PreparedStatement()
  {
    sqlite3_finalize(m_stmt);
  }

  /** \brief implicitly converts to sqlite3_stmt* to be used in SQLite C API
   */
  operator sqlite3_stmt*()
  {
    return m_stmt;
  }

private:
  sqlite3_stmt* m_stmt;
};

This helper should be placed in util/sqlite-helper.hpp so that it can be reused in other modules.

I think it is a good idea. One question, would this be public, or local to PibSqlite3? I do not want to make it public, because its implementation details, and it is not related to ndn. However, we use sqlite3 in several places, so it may cause some code duplication.

Answer to note-8:

The header is public, but it can be marked as "implementation detail" in Doxygen (similar to common.hpp which is marked as "implementation detail").