Range for loop for statements#545
Conversation
fbb99e8 to
83bb827
Compare
300d023 to
c996909
Compare
SRombauts
left a comment
There was a problem hiding this comment.
Thanks, all three earlier points are addressed. Two small follow-ups below, both non-blocking.
| /// Post-increment: advance to the next row, return a copy of the iterator before advancing. | ||
| SQLITECPP_API RowIterator operator++(int); | ||
|
|
||
| /// Return true when two iterators point to the same row. |
There was a problem hiding this comment.
The implementation is fine, but the doc overstates it. It compares the Statement*, not the row, so two live iterators over the same statement always compare equal regardless of position. That is correct for a single-pass stream iterator (it is how std::istream_iterator works, and your @warning already rules out two live iterators). Only the wording needs to match:
/// Return true when both iterators refer to the same statement, or are both the end sentinel.| return *this; | ||
| } | ||
|
|
||
| Statement::RowIterator Statement::RowIterator::operator++(int) |
There was a problem hiding this comment.
Worth flagging: the copy this returns shares the one live cursor, so *it++ gives the row after the increment, not before. This is inherent, not a bug to chase down. operator* hands back the single Statement&, Statement is move-only (Statement.h:79), and sqlite overwrites the row data on the next step, so the returned copy cannot hold the old row. std::istream_iterator gets away with *it++ because it caches its value by copy; here the value is a live cursor, so we cannot.
Caching the row would mean copying every column into the iterator, which is not worth it. The honest signal for a single-pass iterator is to return void from post-increment:
void RowIterator::operator++(int) { ++*this; }That still satisfies std::input_iterator (weakly_incrementable only requires i++ to be a valid expression, with no constraint on its return type), and it turns auto old = *it++; into a compile error instead of a silent wrong-row read. If you would rather keep the conventional copy-returning form, add a @warning saying the iterator is single-pass and *it++ reflects the advanced row.
There was a problem hiding this comment.
You're correct, a good catch
Returning a void, and producing a compile error is much safer than making a warning that could be missed
|
Hey @Alvov1, thanks a lot for your contribution! |
|
One more thought, and only if you are up for it: this feature is genuinely nice, and right now the only place it shows up is the tests. Would you be open to adding a small bit of documentation so people actually discover it? The natural spot is the query loop snippet in No worries if you would rather I add it after merging, just let me know which you prefer. |
|
Hi! I'll make it in a couple of days. Didn't forget |
Closes #181
Adds a nested
RowIteratorclass toStatementthat enables range-based for loops over query results:Notes:
begin()callsreset()automatically, so re-iterating the same Statement always starts from the beginningreset()RowIteratorsatisfiesstd::input_iterator_tagwith full iterator traits