Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Option Feature Request since I Can't put null value to settings on Android #12

Closed
halkar opened this issue Jan 12, 2015 · 16 comments

Comments

@halkar
Copy link

halkar commented Jan 12, 2015

Hello!
I can't addorupdate having null as value (this causes NRE).
Is that by design?

Thank you!

@jamesmontemagno
Copy link
Owner

This is by design. I must call Type typeOf = value.GetType(); to get the type of the value so if value is null it will throw an exception and there isn't too much I could do about it at that point. Additionally, I could not guarantee the underlying settings method allows null values on all platforms.

@halkar
Copy link
Author

halkar commented Jan 13, 2015

Actually I was thinking about removing record if value is null. Cause at the moment I have to use "special values" to "remove" record.

@jamesmontemagno
Copy link
Owner

Ahhhh interesting. I might be able to introduce a remove into the API. Let me see what I can do

@jamesmontemagno jamesmontemagno changed the title Can't put null value to settings on Android Remove Option Feature Request since I Can't put null value to settings on Android Jan 13, 2015
@jamesmontemagno
Copy link
Owner

So a question @halkar would be can't you just set it back to the default value and check against that? Adding a remove method would simply mean that next time you get the value back it would be the default anyways.

Also are you storing strings? That would be the only nullable type in there....

@halkar
Copy link
Author

halkar commented Jan 13, 2015

@jamesmontemagno in my case it was nullable DateTime. So yes, I'm using special value for now, but being able to check value against null instead of DateTime.MinValue will be a plus ;) And doesn't matter how it would be implemented as "Remove" method or by setting null as a value.

@halkar
Copy link
Author

halkar commented Jan 13, 2015

Actually I'm ok to implement that by myself if you agree that it will be useful.

@jamesmontemagno
Copy link
Owner

@halkar how everything is implemented it would get the default value of Null next time you asked for it which would most likely be DateTime.MinValue. All of them are stored as a long though in the settings.

I am just a bit confused about why you need to remove a value.

The issue here is that when I first released this plugin a long time ago the API was probably a bit wrong:

bool AddOrUpdateValue(string key, Object value);

this is the current method, and I need to get the type of value to then set it correctly. If it is null then I can't figure out how to store your setting so it throws an exception, which is how I designed it originally. If you think about it it makes sense.

Ideally though I could have originally made it:

bool AddOrUpdateValue(string key, T value = default(T));

This probably would have been the better way of doing it, but a bit late right now.

The real debate that I can solve it in 1 of 3 ways:

1.) Add a remove method for a key (however next time you call GetValueOrDefault it will return your default
2.) If you set value to null then automatically remove the key (however next time you call getvalueordefault it will return the default)
3.) Leave it as is.

@halkar
Copy link
Author

halkar commented Jan 13, 2015

@jamesmontemagno I'm confused now too %)
First default(DateTime?) == null and I'd like to check if value is null and not DateTime.MinValue.
Second both 1 and 2 approaches are good for me but 1 is more straightforward, obvious and less risky.
If you don't like the idea - that's ok. No pressure ;)

@jamesmontemagno
Copy link
Owner

I don't believe that the API will actually support DateTime? and in fact I do Convert.ToDateTime(value) and that would be DateTime.MinValue.

The other issue is that when I call:
var defaults = NSUserDefaults.StandardUserDefaults;

defaults.ValueForKey(new NSString(key));

This method on iOS ValueForKey will return null if the value doesn't exists. So you can't really save null....

@jamesmontemagno
Copy link
Owner

So I think that might work if you set the default to null to begin, which means that it will return null as the default. Now once you set it you could try to set it to null, but that will result in a DateTime.Min. However, you could remove the key, and then if you call Get it will return null.

Odd edgecase, but crafting some things. Need to ensure I don't break apps and that I like that API if I change it.

@halkar
Copy link
Author

halkar commented Jan 14, 2015

Sure. I'm trying to implement that at the moment by myself. Thank you!

@jamesmontemagno
Copy link
Owner

@halkar so if you pull down this branch and test it you will see my changes to the API:

https://github.com/jamesmontemagno/Xamarin.Plugins/tree/settings_remove_option

I still need to do testing and put the API under review.

@halkar
Copy link
Author

halkar commented Jan 16, 2015

@jamesmontemagno looks good!

@jamesmontemagno
Copy link
Owner

I have released this as an alpha: https://www.nuget.org/packages/Xam.Plugins.Settings/2.0.0-alpha

Please test and let me know

@halkar
Copy link
Author

halkar commented Jan 21, 2015

Tested, everything is fine with me. Thank you!

@jamesmontemagno
Copy link
Owner

1.5.0 is official now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants