cloudcrate / BlazorStorage

A library for Blazor local and session storage support.
MIT License
113 stars 19 forks source link

GetItem, IEnumerable<T>, and nulls #3

Closed scottsauber closed 6 years ago

scottsauber commented 6 years ago

If I have the following code:

@foreach (var item in ToDoItems)
{
    <li>@item.Name</li>
}

@functions {
    List<ToDoItem> ToDoItems = new List<ToDoItem>();

    protected override void OnInit()
    {
        ToDoItems = Storage.GetItem<List<ToDoItem>>("SomeKey");
    }
}

And there is no Local Storage set at SomeKey, then ToDoItems will be set to null, because here if there is no key found it is using default(T) and IEnumerables default to null and my foreach will bomb with a Null Ref Exception. So I have to do something to null check against this such as:

    protected override void OnInit()
    {
        var existingItems = Storage.GetItem<List<ToDoItem>>(storageKey);

        if (existingItems != null)
            ToDoItems = existingItems;
    }

It would be cool if BlazorStorage would default to an empty collection if T is an IEnumerable. Happy to send a PR if you think it's worth doing, or if you want to implement yourself, that's perfectly fine too. Was thinking something like

bool isNullOrEmpty = string.IsNullOrEmpty(json);
if (isNullOrEmpty)
{
     if (typeof(IEnumerable).IsAssignableFrom(typeof(T)))
     {
        return new T();
      }
      return default(T);
}
return Json.Deserialize<T>(json);  

If you don't think this is worth doing, no big deal, the null check isn't the end of the world, it just wasn't what I was expecting.

Also just FYI I'll be demoing your library at KCDC on Friday. Great work!

Thanks.

LunicLynx commented 6 years ago

Hey @scottsauber great idea! But i see some potential issues with it, the expectation from the signature is that it should return null in this case. Also one could really want to know if it is null or an empty collection. But how about another overload, that allows to specify a default value that should be returned in case of a null or empty value?

Something like this Storage.GetItem("Key", new List<ToDoItem>()) or Storage.GetItem("Key", () => new List<ToDoItem>())

Would that be something of interest to you?

Another idea for your scenario might be to create an extension with exactly your logic in it.

public static T GetItemOrNew<T>(this Storage storage, string key)
    where T : new()
{
    var json= storage.GetItem(key);
    if (string.IsNullOrEmpty(json))
    {
            return new T();
    }
    return Json.Deserialize<T>(json); 
}

Thanks for the support, let me know how it went! And if there is video coverage i would be interested to see it!

scottsauber commented 6 years ago

True, I guess you may treat no key exists in local storage differently than "the key exists, but nothing is in here."

Sounds good, I'll let you know on the video. It was recorded, so I assume it'll be up at some point.

Basically demo'd using it with ToDoMVC and this repo: https://github.com/scottsauber/BlazorToDoMVC