Storing and retrieving and Preferences

[ronsmits]

Please review and comment

[edvin]

I try to remember to put the gpg signing back in deploy, but it fails when I deploy, so I have to set it to install before every deploy. That's why it keeps changing :)

I think the preference implementation has some issues, and I don't like the loose _nodename variable.

Since Preferences is loaded via lazy, I suspect you're not able to change the nodename after your first call to it. If you call preferences("somename") and put some preferences in it, then call pereferences("someothername") you'll still be operating on the same instance, right?

Maybe it's better to drop the lazy initialization and just perform the instantiation in the preferences block each time?

[ronsmits]

I will change it, the first version I showed you did something like that.

[edvin]

Perfect :) Add to CHANGELOG as well :)

[ronsmits]

So, I have been thinking about this. and if you want this after this, I will do it. But :)

the java preferences are meant for an application, not for components like the current config is. To be honest I cannot find a decent usecase where I would store application wide preferences over different files. So making the preferences be able to do that sounds to me a bit like overkill.
_nodename should be in an object I agree with that fully. The lazy initialisation can be be removed but looking at my writing about I wonder if it is needed.

[edvin]

I agree that you probably don't want to put app wide preferences is different files. For config this makes sense, since you configure per-component settings like window positions and ui state for example.

The only problem I have with lazy init is that you can call it with a name argument that is actually just discarded if you have already initialised it. Apart from that, I'm good with it.

I have never used Preferences myself, and you could just as well use config in a single class (think ConfigController), but as you pointed out, the files will be put in the folder you started the app from, so Preferences would be better for apps that are distributed without an installer.

There might be something I'm missing here, I'll play with it tomorrow and get back to you, OK?

[edvin]

I've looked at this again now. To solve the problem I mentioned (calling preferences with different node names are simply ignored, everything will go into the first node name), I propose changing the implementation to this:

fun preferences(nodename: String? = null, op: Preferences.() -> Unit) {
    val node = if (nodename != null) Preferences.userRoot().node(nodename) else Preferences.userNodeForPackage(FX.application.javaClass)
    op(node)
}

No nodename will result in the default, and any other node name will be returned correctly. What do you think?

[edvin]

@ronsmits did you have a chance to look at this? :)

[ronsmits]

Sorry I have been very occupied with other stuff these days. I tested this and yes this is fine. I will update my clone and push it

[edvin]

Cool! Are you happy with it now, should I merge?

[ronsmits]

Yes you can merge

[edvin]

Merged manually :)