1

Scenario

In an MVVMLight based app I use a couple of views and services to do some work time calculation. Among others there are the following services...

  • DataService: As the name suggests, used to get the data from the database. Because it's I/O operations per nature the methods are using async/await.
  • Contract: This is a CPU bound service that actually calculates the work times (e.g. for a given day, week, month, ...)

Now, let's have a look at the following method (which is located in the Contract service):

public TimeSpan GetWorkTimeForDay(Date date)
{
    // Check if that result was already calculated before and is still
    // valid. In this case, load it and return it
    if (ContractCalculationCacheResults.Contains(date))
        return ContractCalculationCacheResults.Get(date);

    var finalWorkTime = TimeSpan.Zero;

    // Get the work day from the database in a synchronous fashion
    var workDay = DataService.GetWorkDay(date);

    // if there is a work day in the database then finally calculate the work time
    if (workDay != null)
        finalWorkTime = GetWorkTimeForDay(workDay);

    // cache the result
    ContractCalculationCacheResults.Put(date, finalWorkTime);

    // return the final result
    return finalWorkTime;
}

This method is located in the Contract service. It has the dependency to the DataService to get the required data for the calculations.

As you can see, DataService.GetWorkDay() is synchronous at the moment. This is how the database got accessed for quite some time. Now, while porting the app from WP8 Silverlight to UWP I wanted to clean up the code and changed the data service so that it only provides async methods instead of synchronous methods. But now I'm struggling with combining two actually separate services where ones nature is synchronous while the others is asynchronous.

Problem

I need to update the method GetWorkTimeForDay() (and many others that look similar) to use the new DataService.GetWorkDayAsync() method which, as the name applies, is defined as follows:

public async Task<WorkDay> GetWorkDayAsync(Date dateKey)

However, this leads to many open questions as suddenly GetWorkTimeForDay should become an async method as well, even though its nature is CPU bound. So this won't make sense.

Possible Solutions

1. Make GetWorkTimeForDay an async method

The result would be something like this:

public async Task<TimeSpan> GetWorkTimeForDay(Date date)
{
    ...

    // Get the work day from the database in a synchronous fashion
    var workDay = await DataService.GetWorkDayAsync(date);

    ...
}

While this looks easy, it has a lot of implications on the rest of the code. Simply because that everywhere where GetWorkTimeForDay is called I would have to change the code because this method is now async. I would have to await for it and therefore might need to change the calling method as well... and so on. But sometimes it is not required to call GetWorkTimeForDay() asynchronously simply because it is already running in a separate thread or a background task. Let alone all the Unit-Test that exist so far. So there are cases where it won't block the UI.

Furthermore, suddenly the CPU bound calculations become asynchronous which is not how it should work (at least if I understood this post (and others) by Stephen Cleary correctly.

2. Separate the services

So the next solution that came to my mind is to separate the services. In other words, do not have one service depend on another. This would change the signature for GetWorkTimeForDay() like so:

public TimeSpan GetWorkTimeForDay(WorkDay workDay, Date date)
{
    var finalWorkTime = TimeSpan.Zero;

    if (workDay != null)
        finalWorkTime = GetWorkTimeForDay(workDay);

    return finalWorkTime;
}

The calling method would then look similar to this:

public async void Caller(Date date)
{
    ...
    
    if (ContractCalculationCacheResults.Contains(date))
        return ContractCalculationCacheResults.Get(date);
    
    var workDay = await DataService.GetWorkDay(date);
    var workTime = Contract.GetWorkTimeForDay(workDay, date);

    ContractCalculationCacheResults.Put(date, workTime);

    ...
}

The downside, just like for for the other solution, is that all the caller methods would have to be updated (and its callers, ...) even though sometimes this is not necessary. Also a lot of redundant code for the caching and database access will be added.

One more downside, from an MVVM point of view, is probably that suddenly the data is provided to the contract service. So I won't use DI anymore which is more or less against the rules for good reasons. (Here to me the question pops up on how to correctly inject a service into a service)

Last but not least, I would have to remove the caching from the GetWorkTimeForDay method as well. Only when the result was not cached before, I'm gonna ask the database to get the work day and do the calculations afterwards.

3. ?

...

Actual Question

What is a good design to combine an async service together with a non-async service in an MVVM based project?

I want to keep the actual calculations synchronous because they are CPU bound. I want to keep the data access asynchronous because they are not CPU bound but might take some time.

So this post is not about a technical solution on how to wait for the result of await/Task/... . It is more an architectural question about MVVM, services injected into other services, combination of services that are synchronous and asynchronous.

Community
  • 1
  • 1
Stephan
  • 1,791
  • 1
  • 27
  • 51
  • Possible duplicate of [How to call asynchronous method from synchronous method in C#?](https://stackoverflow.com/questions/9343594/how-to-call-asynchronous-method-from-synchronous-method-in-c) – Haukinger Jul 07 '17 at 05:07
  • For me it is more an architectural question instead of getting an answer on how to wait for an `async` method to complete. – Stephan Jul 07 '17 at 05:33
  • `Furthermore, suddenly the CPU bound calculations become asynchronous which is not how it should work (at least if I understood this post (and others) by Stephen Cleary correctly.` This is merely a general recommendation. If you need your method to be asynchronous, it doesn't matter whether it is IO or CPU bound. In the case of a UI application, you really shouldn't do any lengthy computation (whether it is IO or CPU bound) on the UI thread. Using an asynchronous method is the right thing to do – Kevin Gosse Jul 07 '17 at 06:19
  • 1
    Plus, the recommendation is that "you shouldn't make a method async solely because of a CPU bound operation". This is because the computation will completely hog a thread, so it's more efficient to use the current one rather than offloading to another (assuming this is not the UI thread). But in your example, your method is async because of `GetWorkDayAsync`, so because of an IO bound operation. There really is no issue here, just make `GetWorkTimeForDay` async – Kevin Gosse Jul 07 '17 at 06:24
  • If you want to call an async method that returns a value from a synchronous method and NOT block the current thread simply https://msdn.microsoft.com/en-us/library/dd270696(v=vs.110).aspx –  Jul 07 '17 at 13:29

1 Answers1

3

Furthermore, suddenly the CPU bound calculations become asynchronous which is not how it should work...

There is a simple reason behind this: When people see an async method, they usually think, that this method will execute asynchronously and therefor not block the current thread.

Your method would act like this:

//Start...             (On current thread)
//Access the database  (On IO Thread, freeing up the current thread)
//Return to current thread
//Do the calculations  (On the current thread)

What people might not expect is the workload on the calling thread.

I'd go with @kevin-gosse here: Just change it to an async method, because it still has an IO Bound part int it. If you expect other people to call that method, make sure to document that is has an CPU intensive part too.

Stephen Cleary has some great examples on his blog.

Your second solution approach is completly valid too. If you think of your ´Contract´ purely as a calculation engine, that should absolutely not care where it's data comes from. You'd still have a decoupled, testable class.

Kai Brummund
  • 3,538
  • 3
  • 23
  • 33
  • `Just change it to an async method, because it still has an IO Bound part int it. If you expect other people to call that method, make sure to document that is has an CPU intensive part too.` [Exactly](https://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-even-in.html). – Stephen Cleary Jul 08 '17 at 02:35
  • Nice! I added your Post to the answer. – Kai Brummund Jul 08 '17 at 10:16