
Hi Everyone, Again, I wanted to thank Klemens for his determination in developing, improving and submitting for review the Boost.Sqlite library. I also want to thank Mohammad for managing the review. My recommendation is to REJECT the library in its current shape, and encourage the author to resubmit the library in the future, when the issue listed below is addressed. I am *not* well familiar with SQLite, and my review is based on studying the documentation. I haven't played with the library. I see a great value in offering a wrapper over the native SQLite C API that offers clean, automatic, C++-style resource management. However, the way I see it, the library, in its current shape, does not offer this, or at least fails to communicate it in the documentation. This is my motivation for recommending rejection. As a C++ programmer I have two viable options. Either accept that I am dealing with the C interface, and write a resource-managing wrapper myself, or *trust* that a library performs a clean, correct resource management. I do not see a clear indication that class `connection` does the resource management right: 1. It is not documented 2. Constructor parameter take_ownership implies (but this is never documented) that a connection may have and use a connection handle and yet not manage its lifetime. How do I even tell if the connection object I have received does or does not manage the lifetime of the corresponding handle? 3. There are two functions for which it is not clear how they differ: release() ("Release the owned handle.") and close() ("Close the database connection."). What does function valid() check? having called close() or having called release() or having used `take_ownership`? I do not see a clear resource management in the class `statement`. Is `statement` associated with a resource that needs to be managed? The class has a comment "This transfers ownership to the resultset". Ownership of what? What is the full lifetime of this implied resource? Can execute() be called multiple times? If so, do you transfer the same ownership of something to multiple places? I do not see a clear resource management scheme in class `resultset`. I intuitively feel that some life-time-related thing happens each time I call read_next(). But I do not know when and what actually happens. I also do not know in the iterator interface whether it is operator++ or operator* that corresponds to read_next(). Or if the call to begin() also involves calling read_next(). THe documentation is silent about this important aspect. I fail to see how the example provided could be correct: sqlite::resultset rs = conn.query("select * from users;"); do { handle_row(r.current()); } while (rs.read_next()); // read it line by line The first call to `r.current()` is performed unguarded, without any check. How does this work given that the query can potentially return zero results? These things lead me to the conclusion that I would not be able to correctly manage resources if I used this library interface in its current shape. It is possible that the implementation is fine, and this is just the issue with the documentation. But in my opinion this aspect (resource management) is so important that it cannot be left undocumented, and having left it undocumented in the first place may be an indication that not enough thought has been given to the correct design of the resource management. I hope that the next incarnation of the library will address this aspect. Best regards, &rzej;