Improve Code Reuse (Python-style decorator for c#?)


Improve Code Reuse (Python-style decorator for c#?)



I'm moving from python to c#, and one of the things I miss is python-style decorators. If I had repeated code at the top of a load of functions (validation checking etc), I could create a decorator to do it.



I've seen that there are c# decorators of a sort, but they look like they work more on the class level. Although I am a bit confused by them.



Regardless - how would you go about improving code re-use/DRY in this function? All the stuff in the function is common, except the two places marked. Its callback driven Tcp requests to a server, with a block to stop multiple concurrent requests (check for Idle state).


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

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

// Check if the request succeeded
if (request.OK)
{
/**
* Unique code here
*/
}
// Request failed. Call the onError callback if provided
else { onError?.Invoke(request); }
};

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

/**
* Unique code here, that will later trigger the callback
*/
}



EDIT: I wasn't really thinking of this as a code review task, but now I can see it is. Here is the whole class, showing how the states/vars interact. The Server class handles interactions between us (the client) and a webserver to handle game login, and create/join match.



I'm not fixed on any particular structure, but at some point I want to connect UI buttons to simple functions like Server.Login() and Server.JoinMatch(), without needing to spawn messy classes.


Server.Login()


Server.JoinMatch()


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





You might be right. Mod, can you move it?
– Oliver
Jun 29 at 11:08





Mod doesn't like my flagging for attention to move it. So here it will stay. meta.stackexchange.com/questions/2683/…
– Oliver
Jun 29 at 12:06





This sems like a code-review and thus should also go to codereview.stackexchange.com
– HimBromBeere
Jun 29 at 12:08





Indeed, now that you've posted the full (working) code, it makes it a good candidate for codereview.
– Spotted
Jun 29 at 12:15





Moved to new question: codereview.stackexchange.com/questions/197638/…
– Oliver
23 hours ago




1 Answer
1



Depending wether the two unique code must always be used by pair or not I would choose a different approach.



If you want to enforce "pair usage", you can use an abstract class:


public abstract class MyClass
{
public void MyFunction(string apples, Action<TcpRequest> onSuccess=null, Action<TcpRequest> onError=null)
{
// Throw exception if already busy with an operation
if (!state.Has(State.Idle)) { throw new OperationInProgress(); }

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

// Check if the request succeeded
if (request.OK)
{
SomethingUnique1();
}
// Request failed. Call the onError callback if provided
else { onError?.Invoke(request); }
};

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

SomethingUnique2(callback);
}
protected abstract void SomethingUnique1();
protected abstract void SomethingUnique2(Action<TcpRequest> callback);
}



And then implement as many subclasses as needed:


public sealed class MyClassVariant1 : MyClass
{
protected override SomethingUnique1() { /*...*/ }
protected override SomethingUnique2(Action<TcpRequest> callback) { /*...*/ }
}

public sealed class MyClassVariant2 : MyClass
{
protected override SomethingUnique1() { /*...*/ }
protected override SomethingUnique2(Action<TcpRequest> callback) { /*...*/ }
}



If you can't enforce pair usage because one "something unique 1" may be used in pair with many "something unique 2" I would foster a decorative approach:


public sealed class MyClass
{
private readonly Action somethingUnique1;
private readonly Action<TcpRequest> somethingUnique2;
public MyClass(Action somethingUnique1, Action<TcpRequest> somethingUnique2)
{
this.somethingUnique1 = somethinUnique1;
this.somethinUnique2 = somethingUnique2;
}
public void MyFunction(string apples, Action<TcpRequest> onSuccess=null, Action<TcpRequest> onError=null)
{
// Throw exception if already busy with an operation
if (!state.Has(State.Idle)) { throw new OperationInProgress(); }

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

// Check if the request succeeded
if (request.OK)
{
somethingUnique1();
}
// Request failed. Call the onError callback if provided
else { onError?.Invoke(request); }
};

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

somethingUnique2(callback);
}
}



And then


var variant1 = new MyClass(() => { /* ... */ }, (TcpRequest r) => { /* ... */ });
var variant2 = new MyClass(() => { /* ... */ }, (TcpRequest r) => { /* ... */ });



Here the approach is more composable and thus, less restrictive.





I like the first one. I'm trying to implement it now. These functions are members of a class, and the State var is a field of this class. I'm trying to avoid needing to pass in a ref to the parent class in order to set the State var, but I might not have a choice.
– Oliver
Jun 29 at 11:38





@Oliver so you mean that in SomethingUnique1 and/or SomethingUnique2 you have to update the state ? You can declare the state as follow to overcome this problem: protected readonly State state;. Although the design is a bit bad, it's still better than using ref.
– Spotted
Jun 29 at 11:44



SomethingUnique1


SomethingUnique2


state


protected readonly State state;


ref





Yea I think I've narrowed this down a path because I've only showed the function. I will put the whole thing in the question.
– Oliver
Jun 29 at 11:47






By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Comments

Post a Comment

Popular posts from this blog

paramiko-expect timeout is happening after executing the command

paramiko-expect timeout is happening after executing the command Can someone help me why timeout is happening after executing the command .I am trying to SSH to a machine and execute a command.I want to store the result of the executed command . I have referred paramiko-timeout but i didn't get my expected result. import traceback try: import paramiko except ImportError: raise Exception("Please install paramiko , pip install paramiko") import pexpect from paramiko_expect import SSHClientInteraction def main(): HOSTNAME = "minion5.net" USERNAME = "root" PASSWORD = "help@432" PROMPT = "root@*:~$s+" try: client = paramiko.SSHClient() client.load_system_host_keys() client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) client.connect(hostname=HOSTNAME, username=USERNAME, password=PASSWORD) with SSHClientInteraction(client, timeout=20, display=True) as interact: ...
Read more

Opening a url is failing in Swift

Opening a url is failing in Swift breakpointing inside the callback and logging success shows false. I added itms-apps to my plist: LSApplicationQueriesSchemes and put this in my app delegate: func application(_ app: UIApplication, open url: URL, options: [UIApplicationOpenURLOptionsKey : Any] = [:]) -> Bool { return true } func moreTapped() { let url = NSURL(string: "itms-apps://itunes.com/developer/quantum-productions/id979315877") if #available(iOS 10.0, *) { UIApplication.shared.open(url! as URL, options: [:], completionHandler: {(success: Bool) in }) } else { UIApplication.shared.openURL(url! as URL) } } Are you testing on a real device? – rmaddy Jun 21 at 23:57 I try on a device, get "Cannot Connect to App Store". Device is online ...
Read more

Export result set on Dbeaver to CSV

Export result set on Dbeaver to CSV Normally I use Dbeaver for windows and always export my result set like this: run my query --> select the result --> export the result set --> select export to clipboard --> done This step by step puts my result set in my clipboard and I can paste it wherever I want to work with it. The problem is that now I am using dbeaver for mac and this guide is not working. I can go on until the moment that I select my result set like in the image below: exporting data set But once I go further in the process, in the last step I get: no query Note that in "source" it was suppose to show the query that originated the result set, but instead it says just "select. As a result it does't select my result or anything (besides being "successful"). Normally my query would show up there automatically and I couldn't find any option that corrects this problem in the menus. Please help me! Thanks. Aft...
Read more