Updating void Init() in FreshBasePageModel to async Task Init()

[BNoohi]

Updating void Init() to async Task Init() to support both testing and best practices. Current method is used async at times and this can cause issues with void return type.

I've created a branch with changes to this method but there are open questions. The change in FreshBasePageModel makes sense but results in a breaking change for all subclasses. Nothing major though, a small change.

The issue then comes from the ResolverPageModel calls calling Init(). They need to either, 1) be changed to async as well, as I did in the branch, or 2) the call needs to be wrapped in a Task.Run().

1. This works and I already have the code working in the branch with updated sample code. The issue is that the ResolvePageModel call is now async which means the call to load this page can potentially get called twice by a handler if the user hits it more than once before the page resolves.

2. The resolver wouldn't be able to ensure the page is initialized before it's displayed. This isn't terrible as this is pretty much how we are currently using the Init() method when there is an waited async call in the async void.

Branch with changes: https://github.com/BNoohi/FreshMvvm
@rid00z on issues with void Init(): http://www.michaelridland.com/xamarin/freshmvvm-n2-ioc-constructor-injection/ (also a great tutorial video)
MSDN article on async best practices: https://msdn.microsoft.com/en-us/magazine/jj991977.aspx

Thoughts?

[rid00z]

Great thanks. I'm just away at the moment but I will look at this asap.

[bokmadsen]

Will the @BNoohi branch be merged in, maybe as an expansion of the IPageModelCoreMethods interface to avoid breaking changes?

[BNoohi]

@rid00z Waiting on your thoughts on this.

[rid00z]

@BNoohi I'm planning on look at this in the next 5 days, along with some other new features for FreshMvvm. :)

[rid00z]

@BNoohi thanks for your work in doing this, I very much appreciate it.

Hi, so I've actually taken a look at this and the side effects of using this isn't worth the pay off. There's a big ripple effect in the whole Framework/App. In addition we don't really want to be waiting at this point, we went to kick off a background task.

We can solve this problem by using the pattern below. What do you think?

    public override void Init (object initData)
    {
        LongRunningTask ().RunForget ();
    }

    async Task LongRunningTask()
    {
        //show loading screen
        await Task.Delay (5000);
        Contacts = new ObservableCollection<Contact> (_databaseService.GetContacts ());
        //stop loading screen
    }

[BNoohi]

Closed, but open to comment? @rid00z I don't think that pattern actually addresses the issue at hand, which is testing of the method and exception handling of the task.

I provided the code as is for option one, but what about option 2? What were your thoughts on that? It should be similar to what you suggested but allows for MSDN best practices and exception handling.

That is, change the method to an async Task but then the line that calls Init() we call with a Task.Run() allowing it to be a fire and forget. Same as it is now, only it can actually throw if something fails in that initialization.

[rid00z]

Actually that solution addresses all the issues. It's testable because you can test both the init method and the long running task. It works according to MSDN best practices, eg you not doing async void. How do you think it doesn't work according with MSDN practices?

Option 2 just moves the problem, as you need to await Task.Run anyway and if you don't await Task.Run then the logic ordering will not be in sync.

[BNoohi]

You're right, with my point being with option 2 is to not await the Task when calling Init, they end up being similar.The only thing is with Init itself having a void type and the tasks set to forget, it's not easy testing that the Init method calls and handles the tasks. You're right that the task you're calling is testable, the issue is Init itself calling the tasks.

[fcogutierrez]

Hello, what is the solution here? Because the RunForget method does not exist...

[rid00z]

@fcogutierrez please take a look at this.
https://gist.github.com/programmation/6cc84cfce8e296596364

I've also done this in the past.
Task.Factory.StartNew(() => MyTask(), TaskCreationOptions.LongRunning);

[papiermache]

newb question - so if a pagemodel needs to invoke an async method to get its data, how to do if Init itself is not async ?

[Ebsan]

@papiermache Try putting the async calls in an overridden FreshBasePageModel.ViewIsAppearing() method.

protected override async void ViewIsAppearing (object sender, EventArgs e)