Skip to content

Conversation

kirillin
Copy link

Sometimes a separate open method is vital.

@coveralls
Copy link

coveralls commented Oct 11, 2019

Coverage Status

Coverage decreased (-0.7%) to 95.882% when pulling 6e86b71 on kirillin:master into 629497e on SRombauts:master.

@SRombauts SRombauts self-assigned this Dec 29, 2019
@SRombauts
Copy link
Owner

SRombauts commented Dec 29, 2019

Hello, thanks for this!

I am quite reluctant to have this change; it modifies a lot the behavior/semantic od the Database object... It needs more thinking, as well as more testing

Have you noticed CI complains about lesser code coverage with your change?
It would be because you don't test all the new code paths.

Also, your default constructor should really Initialize all variables.

@SRombauts
Copy link
Owner

About the test coverage decrease, you would need to test all cases in the new Database::open() method
(see edit: https://coveralls.io/builds/27254814/source?filename=src/Database.cpp)

But actually, it would be a lot better to have my old constructor call this new open method itself, so you would not need any more unit tests, it would be covered by specific existing ones

@SRombauts
Copy link
Owner

See also #103 dynamically create 'DB' item and use it until close

@tzpeng
Copy link

tzpeng commented Aug 30, 2021

//Add close() method
//Fix sqlite3_close sometimes return SQLITE_BUSY
int ret = sqlite3_close(db);
while (ret == SQLITE_BUSY) {
ret = SQLITE_OK;

sqlite3_stmt *stmt = sqlite3_next_stmt(db, NULL);
if (stmt != NULL) {
	ret = sqlite3_finalize(stmt);
	if (ret == SQLITE_OK) {
		ret = sqlite3_close(db);
	}
}

}

@kirillin kirillin closed this by deleting the head repository Mar 11, 2024
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.

4 participants