4
\$\begingroup\$

A manager for a speedy async saving objects to isolated storage, using serialization from Newtonsoft.Json. A project to play with is here.

public static async Task<T> ReadJsonEx<T>(String filepath)
    {
        if (String.IsNullOrEmpty(filepath))
            return default(T);

        return await await Task.Factory.StartNew(async () =>
        {
            using (var store = IsolatedStorageFile.GetUserStoreForApplication())
            using (var stream = new IsolatedStorageFileStream(filepath, FileMode.Open, store))
            using (var sr = new StreamReader(stream))
            using (var jr = new JsonTextReader(sr))
                return await jr.ReadJsonAsyncTask<T>();
        });
    }

    public static async Task<bool> WriteJsonEx<T>(String filepath, T content)
    {
        if (String.IsNullOrEmpty(filepath))
            return false;

        return await await Task.Factory.StartNew(async () =>
        {
            using (var store = IsolatedStorageFile.GetUserStoreForApplication())
            using (var stream = new IsolatedStorageFileStream(filepath, FileMode.Create, store))
            using (var sw = new StreamWriter(stream))
            using (var jw = new JsonTextWriter(sw))
                await jw.WriteJsonAsyncTask(content);

            return true;
        });
    }

Where extensions are

private static readonly JsonSerializer JsonSerializer = new JsonSerializer { NullValueHandling = NullValueHandling.Ignore, MissingMemberHandling = MissingMemberHandling.Ignore };

    public static async Task<bool> WriteJsonAsyncTask<T>(this JsonTextWriter writer, T content)
    {
        writer.Formatting = Formatting.Indented;
        return await TaskEx.Run(() =>
        {
            try { JsonSerializer.Serialize(writer, content); }
            catch (Exception) { return false; }
            return true;
        });
    }

    public static async Task<T> ReadJsonAsyncTask<T>(this JsonTextReader reader)
    {
        return await TaskEx.Run(() => JsonSerializer.Deserialize<T>(reader));
    }

I'm just wondering if this code is secure and if it can be used in the real app. Is the current approach safe enough?

\$\endgroup\$
4
  • 2
    \$\begingroup\$ These edits can probably earn you some rep if you word them into an answer / "selfie" review ;) \$\endgroup\$ Apr 28, 2014 at 17:23
  • \$\begingroup\$ @Mat'sMug fair enough :) Thanks, would move them there. \$\endgroup\$ Apr 28, 2014 at 18:25
  • \$\begingroup\$ Also, please don't edit the code in the question. I've rolled back Rev 14 → 13. \$\endgroup\$ Apr 28, 2014 at 20:09
  • \$\begingroup\$ @200_success it was almost the same, just with extra CreateFolderIfNecessary() call. Though, i got your point: code samples should stay original. \$\endgroup\$ Apr 28, 2014 at 20:33

2 Answers 2

3
\$\begingroup\$

This is very neat. Just a few code formatting nitpicks:

    if (String.IsNullOrEmpty(filepath))
        return default(T);

    return await await Task.Factory.StartNew(async () =>
    {
        using (var store = IsolatedStorageFile.GetUserStoreForApplication())
        using (var stream = new IsolatedStorageFileStream(filepath, FileMode.Open, store))
        using (var sr = new StreamReader(stream))
        using (var jr = new JsonTextReader(sr))
            return await jr.ReadJsonAsyncTask<T>();
    });

Call me over-zealous, I think it would be best to specify the scopes with curly braces - I like the stacked using blocks though, so like this:

    if (String.IsNullOrEmpty(filepath))
    {
        return default(T);
    }

    return await await Task.Factory.StartNew(async () =>
    {
        using (var store = IsolatedStorageFile.GetUserStoreForApplication())
        using (var stream = new IsolatedStorageFileStream(filepath, FileMode.Open, store))
        using (var sr = new StreamReader(stream))
        using (var jr = new JsonTextReader(sr))
        {
            return await jr.ReadJsonAsyncTask<T>();
        }
    });

And here too:

public static async Task<bool> WriteJsonAsyncTask<T>(this JsonTextWriter writer, T content)
{
    writer.Formatting = Formatting.Indented;
    return await TaskEx.Run(() =>
    {
        try { JsonSerializer.Serialize(writer, content); }
        catch (Exception) { return false; }
        return true;
    });
}

I find code reads better going down than going across:

public static async Task<bool> WriteJsonAsyncTask<T>(this JsonTextWriter writer, T content)
{
    writer.Formatting = Formatting.Indented;
    return await TaskEx.Run(() =>
    {
        try 
        {
            JsonSerializer.Serialize(writer, content);
            return true;
        }
        catch (Exception) 
        { 
            return false; 
        }
    });
}
\$\endgroup\$
6
  • 2
    \$\begingroup\$ As a matter of style, I'd also put the return true; within the catch block to show the entire "happy path". \$\endgroup\$ Apr 28, 2014 at 18:14
  • \$\begingroup\$ @JesseC.Slicer good point - I put it in the try block though ;) \$\endgroup\$ Apr 28, 2014 at 18:15
  • \$\begingroup\$ @Mat'sMug Thanks for a review. However, it is indeed only a matter of style :) \$\endgroup\$ Apr 28, 2014 at 18:29
  • \$\begingroup\$ @VitaliiVasylenko Meh, the downside of reviewing nice code ;) \$\endgroup\$ Apr 28, 2014 at 18:35
  • \$\begingroup\$ @Mat'sMug well.. agree :) \$\endgroup\$ Apr 28, 2014 at 18:48
3
\$\begingroup\$

Talking about code stability, I had found several issues so far:

  1. I tried to call WriteJsonEx() from the Application_Deactivated() to save some data - sometimes it works, but several times it failed to finish writing before app deactivates.

    I found an answer here. In a short: calling async during app deactivation is a bad idea. Sync calls should be used instead (app has several seconds to store data, so sync calls should works fine).

  2. No folder autocreation

    Just call DirectoryExists() and CreateDirectory() if needed. Will update sample in a while.

  3. Is it thread-safe?

    Probably adding lock() would help, should test it deeper.

EDIT: I'm doing some crash-tests here. Will publish the results in this thread either.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.