Server Request Class, Async Callbacks

Question

I have a class that handles interactions with a custom rest/json webserver, for the purposes of handling online game matchmaking etc. I want the class to have very simple public functions like Server.Login() and Server.JoinMatch(), so they can be connected up at the UI layer easily. As you will see from the code fairly quickly, there is a lot of repeated code in each function, which I want to refactor out.

The first part of each function ensures only one operation at once. The next bit defines a callback delegate for when the request is finished (async). And the last part starts the actual request operation.

public class Server
{
    #region Fields
    public string playerName { get; private set; }
    public string playerID { get; private set; }
    public string playerToken { get; private set; }
    public string currentMatchID { get; private set; }
    private Patterns.State<ServerState> state = new Patterns.State<ServerState>();
    #endregion

    public Server()
    {
        state.Add(ServerState.Idle);
    }

    public void Login(string playerName, Action<TcpRequest> onSuccess = null, Action<TcpRequest> onError = null)
    {
        // Throw exception already busy with an operation
        if (!state.Has(ServerState.Idle)) { throw new OperationInProgress(); }

        // Define login callback action
        Action<TcpRequest> loginCallback = delegate (TcpRequest request)
        {
            // Add idle state back in
            state.Add(ServerState.Idle);

            // Check if the request succeeded
            if (request.OK)
            {
                // Store player data in class
                playerName = (string)request.requestJson["player_name"];
                playerID = (string)request.responseJson["player_id"];
                playerToken = (string)request.responseJson["player_token"];

                // Add the logged in state
                state.Add(ServerState.LoggedIn);

                // Call the onSuccess callback if provided
                onSuccess?.Invoke(request);
            }
            // Login failed, call the onError callback if provided
            else { onError?.Invoke(request); }
        };

        // Remove idle state
        state.Remove(ServerState.Idle);

        // Perform request
        Request("login", callback: loginCallback, requestJson: new Dictionary<string, object> { { "player_name", playerName }, { "client_version", "test1" } });
    }

    public void CreateMatch(string matchName, Action<TcpRequest> onSuccess = null, Action<TcpRequest> onError = null)
    {
        // Throw exception already busy with an operation
        if (!state.Has(ServerState.Idle)) { throw new OperationInProgress(); }

        // Define callback action
        Action<TcpRequest> callback = delegate (TcpRequest request)
        {
            // Add idle state back in
            state.Add(ServerState.Idle);

            // Check if the request succeeded
            if (request.OK)
            {
                // Add the inLobby state
                state.Add(ServerState.InLobby);

                // Call the onSuccess callback if provided
                onSuccess?.Invoke(request);
            }
            // Request failed. Call the onError callback if provided
            else { onError?.Invoke(request); }
        };

        // Remove idle state
        state.Remove(ServerState.Idle);

        // Perform request
        AuthenticatedRequest("match/create", callback: callback, requestJson: new Dictionary<string, object> { { "match_name", matchName } });
    }

    public void JoinMatch(string matchID, Action<TcpRequest> onSuccess = null, Action<TcpRequest> onError = null)
    {
        // Throw exception already busy with an operation
        if (!state.Has(ServerState.Idle)) { throw new OperationInProgress(); }

        // Define callback action
        Action<TcpRequest> callback = delegate (TcpRequest request)
        {
            // Add idle state back in
            state.Add(ServerState.Idle);

            // Check if the request succeeded
            if (request.OK)
            {
                // Add the inLobby state
                state.Add(ServerState.InLobby);

                // Set currentMatchID in class
                currentMatchID = (string)request.responseJson["match_id"];

                // Call the onSuccess callback if provided
                onSuccess?.Invoke(request);
            }
            // Request failed. Call the onError callback if provided
            else { onError?.Invoke(request); }
        };

        // Perform request
        AuthenticatedRequest("match/join", callback: callback, requestJson: new Dictionary<string, object> { { "match_id", matchID } });
    }

    private void Request(string resource, Action<TcpRequest> callback = null, Dictionary<string, object> requestJson = null)
    {
        // Start async request, invoke callback when done
    }

    private void AuthenticatedRequest(string resource, Action<TcpRequest> callback = null, Dictionary<string, object> requestJson = null)
    {
        // Add login auth data into the requestJson dict or throw exception if we aren't logged in
        // Call Request()
    }
}

Notes:

• Patterns.State class is basically a hashset that keeps track of what states an object has. When a server transaction is in progress, I remove the Idle state, and add it back in when its done. To prevent concurrent transactions, I check for the presence of the Idle state.
• I am restricted to c#6, .net 4.7 (Unity)

EDIT1: I tweaked the callbacks, so that instead of having one callback in the TcpRequest and having it call other things, I added a callback list for onSuccess and onError. This means I just add extra callbacks as required, instead of redefining a master one each time.

public void Login(string playerName, Action<TcpRequest> onSuccess=null, Action<TcpRequest> onError=null)
    {           
        // Throw exception already busy with an operation
        if (!state.Has(ServerState.Idle)) { throw new OperationInProgress(); }

        // Prepare callback lists
        List<Action<TcpRequest>> onSuccessCallbacks = new List<Action<TcpRequest>>();
        List<Action<TcpRequest>> onErrorCallbacks = new List<Action<TcpRequest>>();

        // Add login callback action
        onSuccessCallbacks.Add(delegate (TcpRequest request)
        {
            // Add idle state back in
            state.Add(MakoState.Idle);

            // Store player data in class
            playerName = (string)request.requestJson["player_name"];
            playerID = (string)request.responseJson["player_id"];
            playerToken = (string)request.responseJson["player_token"];

            // Add the logged in state
            state.Add(MakoState.LoggedIn);
        });

        // Add onSuccess/onError callback args if not null
        if (onSuccess != null) { onSuccessCallbacks.Add(onSuccess); }
        if (onError != null) { onErrorCallbacks.Add(onError); }

        // Remove idle state
        state.Remove(MakoState.Idle);

        // Perform request (using NoAuth method as we aren't logged in yet)
        Request("login", onSuccess=onSuccessCallbacks, onError=onErrorCallbacks, requestJson: new Dictionary<string, object>{ {"player_name", playerName }, {"client_version", "test1" } });
    }

Answer 1

I'm answering instead of just editing the original with updates, as these changes solve my initial problem. I'm still interested to see if anyone comes up with anything else or has any other feedback.

• Moved the idle state check if(!state.Has(ServerState.Idle)) into Server.Request()
• Moved remove idle state state.Remove(ServerState.Idle) to Server.Request()
• Added onResponse and onFinish callbacks to the TcpRequest class, to be called before and after all other callbacks.
• Using delegates instead of Actions or Lists of actions.
• Server.Request() now adds a delegate to add the idle state back to the request.

--

public void Login(string playerName, Action onSuccess=null, Action onError=null)
    {
        // Define onSuccess delegate
        TcpRequest.CallbackDelegate onSuccessDelegate = delegate (TcpRequest request)
        {
            // Store player data in class
            playerName = (string)request.requestJson["player_name"];
            playerID = (string)request.responseJson["player_id"];
            playerToken = (string)request.responseJson["player_token"];

            // Add the logged in state
            state.Add(MakoState.LoggedIn);

            // Call the onSuccess callback
            onSuccess?.Invoke();
        };

        // Define onError delegate to call onError action
        TcpRequest.CallbackDelegate onErrorDelegate = delegate (TcpRequest request) { onError?.Invoke(); };

        // Perform request
        Request(resource: "login", onSuccess: onSuccessDelegate, onError: onErrorDelegate,
                requestJson: new Dictionary<string, object> { { "player_name", playerName }, { "client_version", "test1" } });
    }

private void Request(string resource, TcpRequest.CallbackDelegate onSuccess=null, TcpRequest.CallbackDelegate onError=null, Dictionary<string, object> requestJson=null)
    {
        // Throw exception already busy with an operation
        if (!state.Has(MakoState.Idle)) { throw new OperationInProgress(); }

        // Remove the idle state
        state.Remove(MakoState.Idle);

        // Create onResponse delegate to add idle state back again
        TcpRequest.CallbackDelegate onResponse = delegate (TcpRequest r) { state.Add(MakoState.Idle); };

        // Create onFinish delegate to GC object at the end
        TcpRequest.CallbackDelegate onFinish = delegate (TcpRequest r) { pendingRequests.Remove(r); };

        // Add the request to the pending list
        pendingRequests.Add(new TcpRequest(server: mmServerUrl, resource: resource,
                                           onSuccess: onSuccess, onError: onError, onResponse: onResponse, onFinish: onFinish,
                                           requestJson: requestJson));
    }