Change how Column handle statement pointer to enforce better safety.

Author: Kacperos155

include/SQLiteCpp/Column.h

@@ -50,16 +50,16 @@ class Column
      *
      * @param[in] aStmtPtr  Shared pointer to the prepared SQLite Statement Object.
      * @param[in] aIndex    Index of the column in the row of result, starting at 0
-     * 
-     * @throws Exception is thrown in case of error, then the Column object is NOT constructed.
      */
-    explicit Column(const StatementPtr::TRawStatementPtr& aStmtPtr, int aIndex);
+    explicit Column(const TStatementPtr& aStatementPtr, uint16_t aIndex) noexcept :
+        mStatementPtr(aStatementPtr),
+        mIndex(aIndex), mRowIndex(mStatementPtr->mCurrentStep) {}
 
     Column(const Column&) = delete;
     Column& operator=(const Column&) = delete;
 
-    Column(Column&& aColumn) = default;
-    Column& operator=(Column&& aColumn) = default;
+    Column(Column&& aColumn) noexcept = default;
+    Column& operator=(Column&& aColumn) noexcept = default;
 
     /**
      * @brief Return a pointer to the named assigned to this result column (potentially aliased)
@@ -80,27 +80,27 @@ class Column
 #endif
 
     /// Return the integer value of the column.
-    int32_t     getInt() const noexcept;
+    int32_t     getInt() const;
     /// Return the 32bits unsigned integer value of the column (note that SQLite3 does not support unsigned 64bits).
-    uint32_t    getUInt() const noexcept;
+    uint32_t    getUInt() const;
     /// Return the 64bits integer value of the column (note that SQLite3 does not support unsigned 64bits).
-    int64_t     getInt64() const noexcept;
+    int64_t     getInt64() const;
     /// Return the double (64bits float) value of the column
-    double      getDouble() const noexcept;
+    double      getDouble() const;
     /**
      * @brief Return a pointer to the text value (NULL terminated string) of the column.
      *
      * @warning The value pointed at is only valid while the statement is valid (ie. not finalized),
      *          thus you must copy it before using it beyond its scope (to a std::string for instance).
      */
-    const char* getText(const char* apDefaultValue = "") const noexcept;
+    const char* getText(const char* apDefaultValue = "") const;
     /**
      * @brief Return a pointer to the binary blob value of the column.
      *
      * @warning The value pointed at is only valid while the statement is valid (ie. not finalized),
      *          thus you must copy it before using it beyond its scope (to a std::string for instance).
      */
-    const void* getBlob() const noexcept;
+    const void* getBlob() const;
     /**
      * @brief Return a std::string for a TEXT or BLOB column.
      *
@@ -153,12 +153,12 @@ class Column
      * - size in bytes of the binary blob returned by getBlob()
      * - 0 for a NULL value
      */
-    int getBytes() const noexcept;
+    int getBytes() const;
 
     /// Alias returning the number of bytes used by the text (or blob) value of the column
-    int size() const noexcept
+    int size() const
     {
-        return getBytes ();
+        return getBytes();
     }
 
     /// Inline cast operators to basic types
@@ -230,8 +230,19 @@ class Column
     }
 
 private:
-    StatementPtr::TRawStatementPtr  mStmtPtr;  ///< Shared Pointer to the prepared SQLite Statement Object
-    int                             mIndex;    ///< Index of the column in the row of result, starting at 0
+    /**
+    * @brief Returns pointer to SQLite Statement Object to use with sqlite3 API.
+    * Checks if SQLite::Column is used with proper statement step.
+    * 
+    * @throws SQLite::Exception when statement has changed since this object constrution.
+    * 
+    * @return Raw pointer to SQLite Statement Object
+    */
+    sqlite3_stmt* getStatement() const;
+
+    TStatementPtr   mStatementPtr;  ///< Shared Pointer to the prepared SQLite Statement Object
+    uint16_t        mIndex;         ///< Index of the column in the row of result, starting at 0
+    std::size_t     mRowIndex;      ///< Index of the statement row, starting at 0
 };
 
 /**

include/SQLiteCpp/Row.h

@@ -12,6 +12,7 @@
 #pragma once
 
 #include <SQLiteCpp/StatementPtr.h>
+#include <SQLiteCpp/Column.h>
 
 #include <memory>
 #include <string>
@@ -26,35 +27,17 @@ namespace SQLite
 class Row
 {
 public:
-    Row(TStatementWeakPtr apStatement, std::size_t aID);
+    Row(TStatementWeakPtr apStatement, std::size_t aID) :
+        mpStatement(apStatement), mID(aID) {}
 
     std::size_t getRowNumber() const
     {
-        return ID;
+        return mID;
     }
 
-    /**
-     * @brief Test if the column value is NULL
-     *
-     * @param[in] aIndex    Index of the column, starting at 0
-     *
-     * @return true if the column value is NULL
-     *
-     *  Throw an exception if the specified index is out of the [0, getColumnCount()) range.
-     */
-    bool    isColumnNull(const int aIndex) const;
-
-    /**
-     * @brief Return a pointer to the text value (NULL terminated string) of the column.
-     *
-     * @warning The value pointed at is only valid while the statement is valid (ie. not finalized),
-     *          thus you must copy it before using it beyond its scope (to a std::string for instance).
-     */
-    const char* getText(uint32_t aColumnID) const noexcept;
-
 private:
     TStatementWeakPtr mpStatement;
-    std::size_t ID;
+    std::size_t mID;
 };
 
 }  // namespace SQLite

include/SQLiteCpp/Statement.h

@@ -25,7 +25,6 @@ namespace SQLite
 
 // Forward declaration
 class Database;
-class Column;
 
 
 /**
@@ -379,7 +378,7 @@ class Statement : public StatementExecutor
      *          Thus, you should instead extract immediately its data (getInt(), getText()...)
      *          and use or copy this data for any later usage.
      */
-    Column  getColumn(const int aIndex) const;
+    Column  getColumn(const int aIndex);
 
     /**
      * @brief Return a copy of the column data specified by its column name (less efficient than using an index)
@@ -410,7 +409,7 @@ class Statement : public StatementExecutor
      *
      *  Throw an exception if the specified name is not one of the aliased name of the columns in the result.
      */
-    Column  getColumn(const char* apName) const;
+    Column  getColumn(const char* apName);
 
 #if __cplusplus >= 201402L || (defined(_MSC_VER) && _MSC_VER >= 1900) // c++14: Visual Studio 2015
      /**

include/SQLiteCpp/StatementExecutor.h

@@ -250,11 +250,11 @@ class StatementExecutor
     /**
      * @brief Return a std::shared_ptr with prepared SQLite Statement Object.
      *
-     * @return TRawStatementPtr with SQLite Statement Object
+     * @return TStatementPtr with SQLite Statement Object
      */
-    StatementPtr::TRawStatementPtr getStatementPtr() const noexcept
+    TStatementPtr getStatementPtr() const noexcept
     {
-        return mpStatement->mpStatement;
+        return mpStatement;
     }
 
     /**
@@ -316,7 +316,7 @@ class StatementExecutor
 
     /// Shared Pointer to this object.
     /// Allows RowIterator to execute next step.
-    TStatementPtr mpStatement{};
+    const TStatementPtr mpStatement{};
 
     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()

include/SQLiteCpp/StatementPtr.h

@@ -67,7 +67,7 @@ struct StatementPtr
 
 
 /// Shared pointer to SQLite StatementPtr
-using TStatementPtr = const std::shared_ptr<StatementPtr>;
+using TStatementPtr = std::shared_ptr<StatementPtr>;
 
 /// Weak pointer to SQLite StatementPtr
 using TStatementWeakPtr = std::weak_ptr<StatementPtr>;

src/Column.cpp

@@ -25,90 +25,90 @@ const int BLOB      = SQLITE_BLOB;
 const int Null      = SQLITE_NULL;
 
 
-// Encapsulation of a Column in a row of the result pointed by the prepared Statement.
-Column::Column(const StatementPtr::TRawStatementPtr& aStmtPtr, int aIndex) :
-    mStmtPtr(aStmtPtr),
-    mIndex(aIndex)
-{
-    if (!aStmtPtr)
-    {
-        throw SQLite::Exception("Statement was destroyed");
-    }
-}
-
 // Return the named assigned to this result column (potentially aliased)
 const char* Column::getName() const noexcept
 {
-    return sqlite3_column_name(mStmtPtr.get(), mIndex);
+    return sqlite3_column_name(mStatementPtr->getStatement(), mIndex);
 }
 
 #ifdef SQLITE_ENABLE_COLUMN_METADATA
 // Return the name of the table column that is the origin of this result column
 const char* Column::getOriginName() const noexcept
 {
-    return sqlite3_column_origin_name(mStmtPtr.get(), mIndex);
+    return sqlite3_column_origin_name(mStatementPtr->getStatement(), mIndex);
 }
 #endif
 
 // Return the integer value of the column specified by its index starting at 0
-int32_t Column::getInt() const noexcept
+int32_t Column::getInt() const
 {
-    return sqlite3_column_int(mStmtPtr.get(), mIndex);
+    return sqlite3_column_int(getStatement(), mIndex);
 }
 
 // Return the unsigned integer value of the column specified by its index starting at 0
-uint32_t Column::getUInt() const noexcept
+uint32_t Column::getUInt() const
 {
     return static_cast<unsigned>(getInt64());
 }
 
 // Return the 64bits integer value of the column specified by its index starting at 0
-int64_t Column::getInt64() const noexcept
+int64_t Column::getInt64() const
 {
-    return sqlite3_column_int64(mStmtPtr.get(), mIndex);
+    return sqlite3_column_int64(getStatement(), mIndex);
 }
 
 // Return the double value of the column specified by its index starting at 0
-double Column::getDouble() const noexcept
+double Column::getDouble() const
 {
-    return sqlite3_column_double(mStmtPtr.get(), mIndex);
+    return sqlite3_column_double(getStatement(), mIndex);
 }
 
 // Return a pointer to the text value (NULL terminated string) of the column specified by its index starting at 0
-const char* Column::getText(const char* apDefaultValue /* = "" */) const noexcept
+const char* Column::getText(const char* apDefaultValue /* = "" */) const
 {
-    auto pText = reinterpret_cast<const char*>(sqlite3_column_text(mStmtPtr.get(), mIndex));
+    auto pText = reinterpret_cast<const char*>(sqlite3_column_text(getStatement(), mIndex));
     return (pText ? pText : apDefaultValue);
 }
 
 // Return a pointer to the blob value (*not* NULL terminated) of the column specified by its index starting at 0
-const void* Column::getBlob() const noexcept
+const void* Column::getBlob() const
 {
-    return sqlite3_column_blob(mStmtPtr.get(), mIndex);
+    return sqlite3_column_blob(getStatement(), mIndex);
 }
 
 // Return a std::string to a TEXT or BLOB column
 std::string Column::getString() const
 {
+    auto statement = getStatement();
+
     // Note: using sqlite3_column_blob and not sqlite3_column_text
     // - no need for sqlite3_column_text to add a \0 on the end, as we're getting the bytes length directly
-    auto data = static_cast<const char*>(sqlite3_column_blob(mStmtPtr.get(), mIndex));
+    auto data = static_cast<const char*>(sqlite3_column_blob(statement, mIndex));
 
     // SQLite docs: "The safest policy is to invoke… sqlite3_column_blob() followed by sqlite3_column_bytes()"
     // Note: std::string is ok to pass nullptr as first arg, if length is 0
-    return std::string(data, sqlite3_column_bytes(mStmtPtr.get(), mIndex));
+    return std::string(data, sqlite3_column_bytes(statement, mIndex));
 }
 
 // Return the type of the value of the column
 int Column::getType() const noexcept
 {
-    return sqlite3_column_type(mStmtPtr.get(), mIndex);
+    return sqlite3_column_type(mStatementPtr->getStatement(), mIndex);
 }
 
 // Return the number of bytes used by the text value of the column
-int Column::getBytes() const noexcept
+int Column::getBytes() const
 {
-    return sqlite3_column_bytes(mStmtPtr.get(), mIndex);
+    return sqlite3_column_bytes(getStatement(), mIndex);
+}
+
+sqlite3_stmt* Column::getStatement() const
+{
+    if (mStatementPtr->mCurrentStep != mRowIndex)
+    {
+        throw SQLite::Exception("Column is used after SQLite Statement Object has been changed");
+    }
+    return mStatementPtr->getStatement();
 }
 
 // Standard std::ostream inserter

src/Row.cpp

@@ -19,25 +19,5 @@ namespace SQLite
 {
 
 
-    Row::Row(TStatementWeakPtr apStatement, std::size_t aID) :
-        mpStatement(apStatement), ID(aID)
-    {
-    }
-
-    bool Row::isColumnNull(const int aIndex) const
-    {
-        auto statement = mpStatement.lock();
-
-        return (SQLITE_NULL == sqlite3_column_type(statement->getStatement(), aIndex));
-    }
-
-    const char* Row::getText(uint32_t aColumnID) const noexcept
-    {
-        auto statement = mpStatement.lock();
-
-
-        auto pText = reinterpret_cast<const char*>(sqlite3_column_text(statement->getStatement(), aColumnID));
-        return (pText ? pText : "");
-    }
 
 }  // namespace SQLite

src/Statement.cpp

@@ -119,7 +119,7 @@ void Statement::bind(const int aIndex)
 
 // Return a copy of the column data specified by its index starting at 0
 // (use the Column copy-constructor)
-Column Statement::getColumn(const int aIndex) const
+Column Statement::getColumn(const int aIndex)
 {
     checkRow();
     checkIndex(aIndex);
@@ -130,7 +130,7 @@ Column Statement::getColumn(const int aIndex) const
 
 // Return a copy of the column data specified by its column name starting at 0
 // (use the Column copy-constructor)
-Column Statement::getColumn(const char* apName) const
+Column Statement::getColumn(const char* apName)
 {
     checkRow();
     const int index = getColumnIndex(apName);

src/StatementPtr.cpp

@@ -40,7 +40,8 @@ namespace SQLite
         return sqlite3_step(mpStatement.get());
     }
 
-    StatementPtr::TRawStatementPtr StatementPtr::prepareStatement(sqlite3* apConnection, const std::string& aQuery) const
+    StatementPtr::TRawStatementPtr StatementPtr::prepareStatement(sqlite3* apConnection,
+        const std::string& aQuery) const
     {
         if (!apConnection)
             throw SQLite::Exception("Can't create statement without valid database connection");

tests/Column_test.cpp

@@ -260,16 +260,12 @@ TEST(Column, shared_ptr)
     query->executeStep();
     column0 = query->getColumn(0);
     EXPECT_EQ(true, column0.isInteger());
+
     query->executeStep(); // query is done
-    
-    // Undefined behavior
-    // auto x = column0.getInt();
+    EXPECT_THROW(column0.getInt(), SQLite::Exception);
 
     query.reset();
-
-    // Undefined behavior
-    // auto x = column0.getInt();
-    // bool isInt = column0.isInteger();
+    EXPECT_THROW(column0.getInt(), SQLite::Exception);
 
     EXPECT_STREQ("id", column0.getName());
 }