StoreDateTimeAsTicks in ISQLiteConnectionFactory #213

Closed
hugoterelle opened this Issue Apr 1, 2013 · 19 comments

Projects

None yet

6 participants

@hugoterelle

Hi Stuart,

When using DateTime in database, the efficient way is to use DateTime as int, instead of string. To use that way, ISQLiteConnectionFactory should support the following parameter:

namespace Cirrious.MvvmCross.Plugins.Sqlite
...
public interface ISQLiteConnectionFactory
{
    ISQLiteConnection Create(string address, bool storeDateTimeAsTicks = false);
}

And the changes have to be impacted into each concrete connection factory. Droid example:

namespace Cirrious.MvvmCross.Plugins.Sqlite.Droid
{
    public class MvxDroidSQLiteConnectionFactory : ISQLiteConnectionFactory
    {
        public ISQLiteConnection Create(string address, bool storeDateTimeAsTicks = false)
        {
            var path = Environment.GetFolderPath(Environment.SpecialFolder.Personal);
            return new SQLiteConnection(Path.Combine(path, address), storeDateTimeAsTicks);
        }
    }
}

That way, we can use the factory like this:

        Connection = factory.Create("NurseMobile.db", true);

The parameter will be spread to the SQLiteConnection constructor with the right parameter.

Yes? No? :)

@slodge

Our branch of SQLite-net is currently still frozen in time until I can persuade @praeclarum to support an interface file separately to the code file (hopefully I'll corner him in Texas ;))

Until then I'm not too keen on branching too far from his source - I would love to try to get his tip supported.


On the SQL connection stuff, my personal opinion is that I hate this line:

    Connection = factory.Create("NurseMobile.db", true);

The true is too meaningless.

Adding to a parameter list like that is also not scalable- if you need to add a few parameters you quickly end up with:

    Connection = factory.Create("NurseMobile.db", true, false, true, false);

which is really, truely unreadable and hard to maintain.


With that all said....

If you still want to fashion this as a pull request, then please do, but I'd encourage you to add options like:

    Connection = factory.Create("NurseMobile.db", new ConnectionOptions() { 
        DateFormat = DateFormat.AsTicks 
    });

or similar.

And I would be happy to push that sort of change back to SQLite-net too.

If I can't persuade sqlite-net to support praeclarum/sqlite-net#135 soon, then I guess we will end up branching forever - which would be a shame :(


PS> It may just be a Belgo-UK thing, but I'm not at all sure what 'Yes? No? :)' means - please, just spell out what you're asking if you can. Assume I'm on a ski holiday and drinking and eating too much understand what you are asking. 'Yes? No? :)' is cool for chat rooms, but this place is for issues. Thanks :)

@hugoterelle

My proposition is not focused on the sqlite-net library but on yours. The original sqlite-net library is using that way of initialization (the boolean true/false option). So, we only change your interface, not the original library.

But as you wrote, I will wait to know if your proposition is accepted by praeclum (I suppose the async features should be supported also with the interface) to go further

@praeclarum
@hugoterelle

Hello,
I don't want something special. I just followed the discussion on SQLite-Net issues about PCL/MvvmCross "fork" plugin, and repeated the discussion on async not supported on mvvmcross (due to droid/touch).

But we have one major problem in mvvmcross/plugin is that new enhancements and bug fixings are not updated by your corrections.

Maybe you should reconsider your position when talking about sqlite-net PCL version as your main project?

Hugo

@slodge

@praeclarum let's discuss in Texas. I like the single file thing, and I'm sure we can work out a #region type build process if we have to... But I've got a feeling that xam studio components and nuget mono* support might make the single file approach secondary... (but maybe 2014 :))

@h2oman

I've added something similar in my fork of MvvmCross, as I've also been storing DateTime as Ticks in my SQLite databases. I also store database files in a different path, so have some need to override the default behaviour for that as well.

I'll implement something similar to the ConnectionOptions that Stuart suggested and then submit a pull request with the changes.

@h2oman

I've implemented a ConnectionOptions parameter in my fork if anyone needs something to work with for now: h2oman@67fe7ef It currently only supports Touch, but will do the other platforms soon.
We'll see where @praeclarum is going with SQLite-Net and if there will be anything happening on that front to facilitate MvvmCross.

@hugoterelle

Hi @praeclarum , Hi @slodge ,

Are you back from Texas with a solution/discussion about sqlite-net integration into mvvmcross?

@slodge

I'm in a plane on the way home

Yes, Frank and I talked.

We will get this done - we had a good agreement.

Only questions now are who does it and when.

Please feel free to volunteer

@h20man I might borrow your impl as a stopgap - thanks

@hugoterelle

Ok, I might be volunteer if you give me some more explanations and time to do the work

@zleao

Is there any news regarding this issue?
The support for SQLite.Async in the MvvmCross plugin would be great.
I can give an 'extra hand' to help on this development...

@slodge

Still frozen - although @praeclarum posted somewhere recently that he's got a single file PCL solution that he's worked on.

I'm 99.9999% sure he'd be happy to have someone else do work for him - best bet is over on praeclarum/sqlite-net#135

@zleao

Thanks @slodge. I'll try to contact @praeclarum and see if I can help in any way, to make this implementation work.

@dtodaro-ascendle

I'm interested in this capability as well a) because of the storage efficiency and performance advantage and b) because I'd like to store date/times with millisecond precision. (The current implementation of SQLite-net drops the milliseconds.)

@slodge

Still pending on praeclarum/sqlite-net#135

Although new forks are appearing - eg. - so maybe one of those will drive a future plugin and it's maintenance https://github.com/jarroda/MvvmCrossJarrod?

@slodge

I'm personally still in favour of waiting for @praeclarum and a merge back to the core source - I think there's huge value in getting an 'official central merge' working - especially as they have working unit tests in the core.

However, to allow 'the community' to push forwards with work, I've branched mvvmcross' sqlite-net out into https://github.com/MvvmCross/MvvmCross-SQLite and am opening up nominations for people who want to admin that repo.

No idea how this is going to work... I'm learning as I go...

@slodge

This is now implemented in https://github.com/MvvmCross/MvvmCross-SQLite using a ISQLiteConnectionFactoryEx factory

Closing this now

Still open for nominations for people who want to admin that community sqlite repo.

@slodge slodge closed this Sep 17, 2013
@praeclarum
@slodge

Thanks @praeclarum - easy to fix in the new fork MvvmCross/MvvmCross-SQLite@c1ea4d0 - will aim to do a first release of it 'soon' :) May change SQLite namespace in it too in order to avoid clashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment