SQLiteAsyncConnection Problem

[prasannavl]

This is really a cross-post of the issue in the parent repository. But frankly, I think this fork is better organized, and cleaner in many aspects.

In the wake of creating a better API, I feel the issue's relevance is more here, than the parent.
Cross-posted Issue: praeclarum#352

"The SQLiteAsyncConnection currently, executes the Func each time its performs a query. Now, this in most scenarios could be ideal, but the problem is how it manages its connection.

The only reason for the design decision to executes the Func that I can think of, instead of a connection directly is so that that this could possibly create a new connection. When we're providing a direct connection, it might as well be done without a Func. It just becomes an unnecessary indirection.

But what's alarming is that if you indeed provide a Func that creates a new connection, now a new problem is created. The connection lifetime management becomes tedious. Since the there's no way to close the connection without holding on to the connection itself externally. And if it does create a new connection, the lock becomes redundant.

Now, the only way to really make use of the Async properly is to pass in the same connection through the func, while both the connection, and the async wrapper being managed explicitly. This opens up more responsibilities for the class than it ought to be dealing with it.

I feel the entire of Async functionality here is misleading and creates more problems (or at-the-least encourages anti-patterns) than it really solves. Perhaps, pull it out of the stable packages for now?

PS: I also feel the ConnectionPool is misleading. Its not really a pool. It just look up a dictionary and manages a single connection for each string. Its no pool of connections."

[oysteinkrog]

I agree that the current design is not great, but cleaning it up too much has some major downsides in the compatibility department.
The func stuff should probably be changed, and I don't really see how ConnectionPool is supposed to work at all (I've kept it to maintain compatablilty).
Perhaps we can start by using the [Obsolete] attribute on types that we want to remove in the future?
As I've noted in the readme the entire async wrapper is not the best idea since the underlying operations are synchronous, but I know a lot of people is using it and thus I don't want to remove it either.

[prasannavl]

Makes sense. But perhaps, a cleaner branch for a v2, may be?

And regarding async, I don't see how a truly asynchronous version can be done, unless SQLite C API itself provides one. So, I guess starting a separate task on a given TaskScheduler is as good as it gets for now. (Taking that into consideration, since all operations are always thrown back into the taskscheduler, the reason why its a bad practice may not apply.) Regardless, I think this may have to be re-designed quite a bit. Because, the locking could be potentially redundant, in case of spanning new connections, but cannot entirely be removed as well. If a task scheduler has a complex synchronization context, then a lock might just be needed. Actually, I think the responsibility of who does the synchronization has to be revisited and cleared up first. If the library is going to handle it, then the better approach would possibly be to have two different classes, one with locking, and one without (Cons: Maintainability). Or the other way would be to completely get rid of it simple using the SQLiteConnection for async, and move the responsibility out of the library for complex task schedulers.

With the Pool, as I see, the so-called ConnectionPool is not really used anywhere. Probably an artifact of a premature class that was supposed to be in a different branch? Anyway, as I see it, if that's marked as obsolete, and then if a real Pool is implemented, it'll have to be renamed. Personally, I think it would just be better to be rid of it for now, since it doesn't make any sense at this point anyway.