0

I have a client/server architecture rolled into the same executable project. It also supports user-code access via script. As such, in my code there are a lot of checks on critical methods to ensure the context in which they are being called is correct. For example, I have a whole lot of the following:

public void Spawn(Actor2D actor)
{
    if (!Game.Instance.SERVER)
    {
        Util.Log(LogManager.LogLevel.Error, "Spawning of actors is only allowed on the server.");
        return;
    }
    //Do stuff.
}

I would like to cut down on this duplication of code. Does there exist something in C# what would give me the functionality to do something like:

public void Spawn(Actor2D actor)
{
   AssertServer("Spawning of actors is only allowed on the server.");
   //Do stuff.
}

Even a generic message like "[MethodNameOfPreviousCallOnStack] can only be called on the server." would be acceptable. But it would have to also return from the caller as well (in this case Spawn()), as to function like an abort. Similar to an assert, but instead of generating an exception just returns. Thanks!

selkathguy
  • 1,171
  • 7
  • 17
  • 2
    Sounds like you *should* be generating an exception. If someone asks to do something they're not allowed to do **YELL AT THEM** rather than just passive aggressively ignoring them. – Servy Apr 07 '14 at 20:58
  • @Servy, the issue comes when I go to write the scripts. I would *much* rather see yellow error messages in my console alerting my that scripts are loading in the wrong context, rather than chase after inconsistent script execution context problems. – selkathguy Apr 07 '14 at 20:59
  • At some level where you are dispatching the entire script for execution it may make sense to catch the exception and display an error to the user and not crash the program. That's entirely possible using exceptions. – Servy Apr 07 '14 at 21:01
  • @Servy hmm, I already have something like that in usage, where if a call from Lua back into C# (like from a script) will generate an ScriptException that is printed gracefully in the console. You've promped me to reconsider whether or not this will be a problem. – selkathguy Apr 07 '14 at 21:07
  • Have you considered using AOP? See my answere [here](http://stackoverflow.com/questions/22920095/how-to-check-for-null-parameter-values); I could almost copy and paste it as an answer to your question. – Daniel Brückner Apr 07 '14 at 21:08

3 Answers3

2

You should consider going up another level of abstraction and add metadata to the method to describe these constraints:

[ServerOnly]
public void Spawn(...)
{
    ...
}

Then use a AOP library like Dynamic Proxy to intercept calls to the method. If the method has the [ServerOnly] attribute, then you can check the context you are running in then return out if it is incorrect.

Chris Pitman
  • 12,990
  • 3
  • 41
  • 56
  • 1
    Isn't this a really heavy hack just to save a programmer from writing two lines of code? Not to mention the need to have a pre-build process added to the build script. – Reactgular Apr 07 '14 at 21:10
  • 1
    The OP explicitly says he wants to cut down code duplication, and dosen't specify any performance requirements. I really think the AOP approach would suit a nice solution to the problem. With a framework like PostSharp, you wont even need to decorate at the method level, you could decorate at the namespace level and save yourself time – Yuval Itzchakov Apr 07 '14 at 21:47
  • I appreciate both of your comments, and this totally works, but as performance is an issue, it will force me to weigh the costs and benefits to suit my application best. Sort of an "is it worth it" question now. Thank you @Chris for the resource! – selkathguy Apr 07 '14 at 21:54
1

This approach will get you pretty close:

public void RunOnServerOnly(Action execFunc, string errorMessage)
{
    if (!Game.Instance.SERVER)
    {
        Util.Log(LogManager.LogLevel.Error, errorMessage);
    }
    else
    {
       execFunc();
    }
}

Then you call it:

RunOnServerOnly(() => Spawn(newActor), "Spawning of actors is only allowed on the server.");

Explanation:

To get rid of the duplicated code, you have one function that performs the check and logging. You give it an action (generic delegate) to perform if the check passes. It runs the if statement, logs if it isn't on a server, and otherwise just runs the function.

I would tend to agree that exceptions are probably the better route to go, but the above meets your requirements.

BradleyDotNET
  • 60,462
  • 10
  • 96
  • 117
  • While I might end up with another solution, and it will require some additional tinkering, +1 this may be useful in the future with the strange abstraction and interoperability I'm dealing with. – selkathguy Apr 07 '14 at 21:08
0

If possible add a ServerContext object that describes the current server instance to the method argument list.

public void Spawn(ServerContext context, Actor2D actor)
{
// do stuff
}

This makes it difficult for the caller to execute this method without a valid context. In that way you are enforcing the rule at compile time.

Reactgular
  • 52,335
  • 19
  • 158
  • 208
  • If called on the client, this will generate a `NullReferenceException` for the nonexistent `ServerContext`, which will break the client unless I surround all calls to `Spawn()` (and everywhere else i need to make this check) in try/catch blocks. I see your point, but I would prefer a solution in which the problem is gracefully handled at runtime. – selkathguy Apr 07 '14 at 21:03
  • @selkathguy This "Spawn" library has a dependency upon the server, but the architecture does not check if this dependency is supported. As a result your testing this at run-time. I would figure out why a "client" could have loaded "Spawn" in the first place. – Reactgular Apr 07 '14 at 21:08