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" } });
 }

• VisualMelon: Is the class itself meant to be thread-safe, or are you just worried about asynchronous call-backs? (i.e. are you intending to address the concern of these methods being called concurrently from multiple threads?)
• Oliver: There are a few threads about, but the UI code that will trigger these functions will be a single thread so its not such a worry. You can in theory fire off several Join or CreateMatch requests, before any results come back and this is likely to cause issues. But there is no reason the "one at a time buddy" lock needs to actually be in here. It could be in the UI layer. It just seemed to make sense this way.

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));
 }