Is there a reason to make every WCF call Async?

Question

Is there a reason to make every WCF service call Async?

I was having this discussion with my partner. He would like to make every WPF service call Async to avoid locking the UI (it is a desktop WPF application). I am against this idea. I do not feel an Async call is needed in most cases, and when it is needed both the RequestingClass and the DataManager should be coded specifically to handle the Async call.

My arguments for this is that is far more code to setup callbacks for everything and it is very confusing. I also think this could cause a performance decrease, although I haven't verified this yet. His argument is that sometimes you are getting a lot of data back and it would lock up the UI, and it isn't that much work to setup the WCF calls like this (he also doesn't find the following code confusing).

We've both never worked with the WCF server before, so I thought I'd give him the benefit of doubt and ask here for some other opinions.

For example:

My Way:

public override User GetById(int id)
{
    return new User(service.GetUserById(id));
}

It locks the UI, UserDataManager, and WCF service channel until the WCF server returns with the User DataTransferObject, however it is easy to understand and quick to code. It would be used for most WCF service calls unless it actually expected a delay in getting data, in which case the DataManager would be setup to handle Async calls.

His Way:

public override void GetById(int id, Action<UserGroup> callback = null)
{
    // This is a queue of all callbacks waiting for a GetById request
    if (AddToSelectbyIdQueue(id, callback))
        return;

    // Setup Async Call
    var wrapper = new AsyncPatternWrapper<UserDTO>(
        (cb, asyncState) => server.BeginGetUserById(id, cb, asyncState),
        Global.Instance.Server.EndGetUserById);

    // Hookup Callback
    wrapper.ObserveOnDispatcher().Subscribe(GetByIdCompleted);

    // Run Async Call
    wrapper.Invoke();
}

private void GetByIdCompleted(UserDTO dto)
{
    User user = new User(dto);

    // This goes through the queue of callbacks waiting 
    // for this method to complete and executes them
    RunSelectIdCallbacks(user.UserId, user);
}

Callback queue on base class:

/// <summary>
/// Adds an item to the select queue, or a current fetch if there is one
/// </summary>
/// <param name="id">unique object identifier</param>
/// <param name="callback">callback to run</param>
/// <returns>False if it needs to be fetched, True if it is already being
/// fetched</returns>
protected virtual bool AddToSelectbyIdQueue(int id, Action<T> callback)
{
    // If the id already exists we have a fetch function already going
    if (_selectIdCallbacks.ContainsKey(id))
    {
        if(callback != null)
            _selectIdCallbacks[id].Add(callback);
        return true;
    }

    if (callback != null)
    {
        List<Action<T>> callbacks = new List<Action<T>> {callback};
        _selectIdCallbacks.Add(id, callbacks);
    }

    return false;
}

/// <summary>
/// Executes callbacks meant for that object Id and removes them from the queue
/// </summary>
/// <param name="id">unique identifier</param>
/// <param name="data">Data for the callbacks</param>
protected virtual void RunSelectIdCallbacks(int id, T data)
{
    if (_selectIdCallbacks.ContainsKey(id))
    {
        foreach (Action<T> callback in _selectIdCallbacks[id])
            callback(data);

        _selectIdCallbacks.Remove(id);
    }
}

It does not lock the UI, the DataManager, or the WCF Service Channel, however a lot of extra coding goes into it.

The AsyncPatternWrapper is in our application regardless. It is something that allows us to make Async WCF calls and subscribe a callback event

EDIT We do have a wrapper which we can use from the UI thread to wrap any DataManager call. It executes the Synchronous method on a BackgroundWorker, and executes a callback against the results.

The majority of the extra code is to prevent locking the DataManager and the WCF Service channel.

Answer 1

Your partner is correct; you shouldn't block the UI thread.

As an alternative to async calls, you can also make synchronous calls in a background thread using a BackgroundWorker or the ThreadPool.