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

Row: New class with iterators (WIP) #363

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Kacperos155
Copy link
Contributor

@Kacperos155 Kacperos155 commented Jul 24, 2022

What I'm trying to implement:

  • Iterators
  • Class Row with methods to get:
    • Row number (from 0 to n)
    • Column object
    • Column ID from name
  • Internal code changes:
    • Split row execution from SQLite::Statement - smaller files, better readability
    • Better structure for weak_ptr with sqlite3_stmt
    • Better enforcement of Column usage safety
  • Other:
    • Fix SQLite::Statement move constructor (and re-enable this test)
    • Documenting comments for all new changes
    • Tests coverage for changes and new functionality
    • Many, many small changes...

Examples:

SQLite::Statement query(database, queryStr);

for(const auto& row : query)
{
    for(const auto& column : row)
    {
        std::cout << column.getName() << ": ";
        std::cout << column.getText() << "\t";
    }
    std::cout << '\n';
}
SQLite::Statement query(database, queryStr);

for (auto it = query.begin(); it != query.end(); std::advance(it, 2))
{
    std::cout << "Table row: " << it->getRowNumber() << '\n';
}

std::for_each(query.begin(), query.end(), [](const SQLite::Row& row)
{
    std::cout << row.getColumn(0).getText() << '\n';
});

@Kacperos155
Copy link
Contributor Author

@SRombauts What do you think about this changes?
Should this PR be separeted to smaller ones?

@dougnazar
Copy link
Contributor

Ironically, I'd just done a simple iterator this morning before I saw yours. Mine was no where near as ambitious as yours, just basically a 80 line (mostly boiler-plate) wrapper around Statement.

@Kacperos155 Kacperos155 marked this pull request as ready for review August 2, 2022 01:34
@Kacperos155
Copy link
Contributor Author

Kacperos155 commented Aug 2, 2022

Iterator functionality is now working 😄
I am sure there isn't any API break excluding undefined behavior of Column class after destruction of Statement.
Because of the scale of changes, I require code review to keep working on this.
Also, this needs more test coverage, benchmarking and feedback, but I think any improvements to this should be in next PRs.

@SRombauts SRombauts self-assigned this Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants