Using list of tasks to obtain and cache data

Question

I develop Windows Phone App with Prism framework (MVVM).

I use data caching. To get data I use proxy service. Proxy service creates two tasks:

1. the first task is receiving data from a web service and update data in local database (SqLite).
2. the second task is receiving data from the local database.

The ViewModel launches tasks and displays data on UI.

It works, but I think it can be done much better. Any ideas?

ViewModel

public class UserProfilePageViewModel : ViewModel, IUserProfilePageViewModel
{
 private readonly IUserServiceProxy _userServiceProxy;
 private IUserItemViewModel _userProfile;
 public IUserItemViewModel UserProfile
 {
 get { return _userProfile; }
 set { SetProperty(ref _userProfile, value); }
 }
 public UserProfilePageViewModel(IUserServiceProxy userServiceProxy)
 {
 if (userServiceProxy == null) { throw new ArgumentNullException("The userServiceProxy can't be null"); } 
 _userServiceProxy = userServiceProxy; 
 }
 public async override void OnNavigatedTo(object navigationParameter, Windows.UI.Xaml.Navigation.NavigationMode navigationMode, Dictionary<string, object> viewModelState)
 { 
 _userId = navigationParameter as string;
 await Load(navigationParameter as string);
 } 
 public async Task Load(string id)
 { 
 var taskList = _userServiceProxy.GetUserTaskList(id);
 while (taskList.Count > 0)
 { 
 var t = await Task.WhenAny(taskList);
 taskList.Remove(t);
 try
 {
 var result = await t;
 //update UI 
 UserProfile.Model = result.Result;
 }
 catch (OperationCanceledException) { }
 catch (Exception) { }
 }
 await Task<User>.WhenAll(taskList);
 }
}

ServiceProxy

public class UserServiceProxy : IUserServiceProxy
{
 private readonly IUserApi _userApi;
 private readonly IUserRepository _userRepository;
 private readonly IUserSettings _userSettings;
 private readonly IAuthService _authService;
 public UserServiceProxy(IUserApi userApi, IUserRepository userRepository, IUserSettings userSettings, IAuthService authService)
 {
 if (userApi == null) { throw new ArgumentNullException("The userApi can't be null"); }
 if (userRepository == null) { throw new ArgumentNullException("The userRepository can't be null"); }
 if (userSettings == null) { throw new ArgumentNullException("The settings can't be null"); }
 if (authService == null) { throw new ArgumentNullException("The authService can't be null"); }
 _userApi = userApi;
 _userRepository = userRepository;
 _userSettings = userSettings;
 _authService = authService;
 }
 public List<Task<ResponseWrapper<User>>> GetUserTaskList(string id, CancellationToken cancellationTokenSource = new CancellationToken())
 {
 Func<Task<ResponseWrapper<User>>> getFromWeb = async () =>
 {
 var poco = await _userApi.GetAsync(id, cancellationTokenSource);
 await _userRepository.MergeUser(LocalSQLite.Models.User.FromPoco(poco.Result));
 return poco;
 };
 Task<Task<ResponseWrapper<User>>> outerTaskWeb = Task.Factory.StartNew(getFromWeb);
 Task<ResponseWrapper<User>> innerTaskWeb = outerTaskWeb.Unwrap();
 Func<Task<ResponseWrapper<User>>> getFromLocal = async () =>
 {
 var localUser = await _userRepository.LoadByUserIdAsync(id);
 if (localUser == null) localUser = new LocalSQLite.Models.User();
 return Models.WebApi.ResponseWrapper<Models.User.User>.Wrap(localUser.ToPoco());
 };
 Task<Task<ResponseWrapper<User>>> outerTaskLocal = Task.Factory.StartNew(getFromLocal);
 Task<ResponseWrapper<User>> innerTaskLocal = outerTaskLocal.Unwrap();
 List<Task<ResponseWrapper<User>>> tasks = new List<Task<ResponseWrapper<User>>>()
 {
 innerTaskWeb,
 innerTaskLocal
 };
 return tasks;
 }
}

Answer 1

Let's start with the ServiceProxy.

1. You're misusing ArgumentNullException, if you pass only one parameter it should contain the name of the parameter which value is null.
2. You don't need Task.Factory.StartNew at all, as you just kill all the benefits of task-oriented approach with it. All you need to do is just capture the task, without awaiting on it. As a result you don't need all those .Unwrap() calls as well.
3. Since you're just outputting the collection of different tasks that eventually return the ResponseWrapper<User> object - I would prefer to return the IEnumerable<T> instead of List<T>.
4. And finally - split different approaches of getting the result record into different methods (if not classes) so that the code remains clean. If you use an IoC container you may even want to split the implementation into different classes implementing the same interface, but that would be too much for this example.

As a result we should get something like this:

UserServiceProxy

public class UserServiceProxy : IUserServiceProxy
{
 private readonly IUserApi _userApi;
 private readonly IUserRepository _userRepository;
 private readonly IUserSettings _userSettings;
 private readonly IAuthService _authService;
 public UserServiceProxy(IUserApi userApi, IUserRepository userRepository, IUserSettings userSettings, IAuthService authService)
 {
 if (userApi == null) { throw new ArgumentNullException("userApi", "The userApi can't be null"); }
 if (userRepository == null) { throw new ArgumentNullException("userRepository", "The userRepository can't be null"); }
 if (userSettings == null) { throw new ArgumentNullException("userSettings", "The settings can't be null"); }
 if (authService == null) { throw new ArgumentNullException("authService", "The authService can't be null"); }
 _userApi = userApi;
 _userRepository = userRepository;
 _userSettings = userSettings;
 _authService = authService;
 }
 public IEnumerable<Task<ResponseWrapper<User>>> GetUserTasks(string id, CancellationToken cancellationToken = new CancellationToken())
 {
 yield return GetUserFromWebAsync(id, cancellationToken);
 yield return GetUserFromLocalAsync(id);
 }
 private async Task<ResponseWrapper<User>> GetUserFromLocalAsync(string id)
 {
 var localUser = await _userRepository.LoadByUserIdAsync(id) ?? new LocalSQLite.Models.User();
 return Models.WebApi.ResponseWrapper<Models.User.User>.Wrap(localUser.ToPoco());
 }
 private async Task<ResponseWrapper<User>> GetUserFromWebAsync(string id, CancellationToken cancellationToken)
 {
 var poco = await _userApi.GetAsync(id, cancellationToken);
 await _userRepository.MergeUser(LocalSQLite.Models.User.FromPoco(poco.Result));
 return poco;
 }
}

Now let's look at the Load method. The main issue there is the processing of each task as it finishes. It can easily be done like this (note that both methods are no longer async, as they do not await on tasks):

 private void UpdateUi(ResponseWrapper<User> user)
 {
 //update UI 
 UserProfile.Model = result.Result;
 }
 public Task Load(string id)
 {
 var uiUpdateTasks = _userServiceProxy.GetUserTasks(id)
 .Select(async userTask => UpdateUi(await userTask));
 return Task.WhenAll(uiUpdateTasks);
 }

• Alexandr: Great! Thank you very much! I have little experience in Task programming, your code is very useful for me.
• Alexandr: If I need to perform some action after all tasks (eg to set a flag allDataLoaded), I should to use await keyword before Task.WhenAll?
• almaz: yes (and mark this method as async)