C# Plugin Survey

[Birdboat]

Hello everyone, I'm in the works of creating a C# plugin base for people to use. Right now I'm in the process of wrapping all the native functions. I'd like to get some input from you all. Would you prefer wrapped functions returning a uint(result), or having a "global" variable to store the result(like WINAPI's GetLastError())?

Example return uint:

public void SomeFunction()
{
    string res = string.Empty;
    uint error = TSFuncs.GetClientLibVersion(ref res);
    if (error == Error.ERROR_ok)
    {
        // Do something with res
    }else {
        // Or don't
    }
}

The downside to this is that all the refs might not be popular among the people that use it

Example with the global variable:

public void SomeFunction()
{
    string res = TSFuncs.GetClientLibVersion();
    if (Plugin.Instance.LastResult == Error.ERROR_ok)
    {
        // Do something with res
    }else {
        // Or don't
    }
}

Either one works fine, I'm just curious which one people will like.
Please leave feedback.

[Thomas]

I'd prefer the first solution (return the errorcode).

I'm no C#-developer, but I'd suggest to port it as close as possible to the "original". It makes it easier for people coming from developing in C/C++.
On the other hand, developers using the C# wrapper for the TeamSpeak 3 SDK use this paradigma as well. Maybe this is a good starting point for you, too?

[cfxxl]

I'm for having return codes as well.
Having a global variable for errorcodes would probably cause issues when dealing with multithreading (although I don't know, how thread-safe the client-plugin-api is in general).

[numma_cway]

In my applications, I simply throw errors if the code is neither "ok" nor "database empty result set".

[Birdboat]

I've decided to go with throwing an error, mainly because the .NET wrapper uses this method as well.

[numma_cway]

Why mainly because of that? To the application, it doesn't matter why a certain command fails and it doesn't make any difference. Show it to the user, in nearly 100% of the time, he is the only one who could do something against the error. "database empty result set" is the only error code that I make an exception for, because it's simply annoying and not even used consistently by TeamSpeak (e.g. clientlist returns "ok" even with no clients [note that unlike voice clients, query clients cannot necessarily see members in their own channel, including themselves]).

[Birdboat]

I was going to because the .NET wrapper is a good base to go off of, but I understand the concern. I think that errors should be handled in the code, so that when a exception was thrown, you knew you forgot a check. But after looking at all the errors, I see that accounting for every single one pertaining to your subject probably isn't a good idea. Do you have any suggestions on how to notify the user of an error?

[numma_cway]

I was going to because the .NET wrapper is a good base to go off of, but I understand the concern. I think that errors should be handled in the code, so that when a exception was thrown, you knew you forgot a check. But after looking at all the errors, I see that accounting for every single one pertaining to your subject probably isn't a good idea. Do you have any suggestions on how to notify the user of an error?

I created a GUI application. The GUI makes sure the user only selects stuff that makes sense. If the user initiates an action from the GUI and an error occurs despite the check, the RTL automatically shows the error with the info provided by the server and cancels the current action the user started. Errors that the GUI cannot prevent are mostly arising from users having destroyed their permission system.

[thorwe]

Personal concerns with using exceptions (exclusively):
Not everything that isn't ERROR_ok is exceptional.
Looking at ERROR_ok_noupdate, ERROR_not_connected and whatnot.
The way I see it it'll be quite the effort to differentiate exceptionality and more control-flow type of errors.
I wouldn't want to throw an error for an invalid password, for example, as that's quite a common thing to happen - it's simply a result of a an action.
And all of a sudden one has two things, exceptions for rare, exceptional circumstances, like the initial mentioned getClientLibVersion - and cases when one wants to simply notify control-flow, result type of information - and return values reappear on the scene.
The deal is also, I think, implementing exceptions properly requires intimate knowledge of API quirks, let's take getClientID, one of the most simple functions out there.

void ts3plugin_onClientMoveEvent(uint64 serverConnectionHandlerID, anyID clientID, uint64 oldChannelID, uint64 newChannelID, int visibility, const char* moveMessage)
{
// Get My Id on this handler
anyID myID;
if((error = ts3Functions.getClientID(serverConnectionHandlerID ,&myID)) != ERROR_ok)
{
TSLogging::Error("(ts3plugin_onClientMoveEvent)",s erverConnectionHandlerID,error);
return;
}
}

Now, given that we receive a client move event on a server connection handler, we must be connected hence not getting the client id is exceptional, right?
Well, not really, that would throw everytime the user disconnects from a server, since:

unsigned int error;
if (newChannelID == 0) // When we disconnect, we get moved to chan 0 before the connection event
{ // However, we aren't able to get our own id etc. anymore via the API for comparison
int con_status; // Therefor, unless we cache our ids for no other benefit, this is a workaround by filtering those out
if ((error = ts3Functions.getConnectionStatus(serverConnectionH andlerID,&con_status)) != ERROR_ok)
{
TSLogging::Error("(ts3plugin_onClientMoveEvent)",s erverConnectionHandlerID,error);
return;
}
if (con_status == STATUS_DISCONNECTED)
return;
}

// Get My Id on this handler
anyID myID;
if((error = ts3Functions.getClientID(serverConnectionHandlerID ,&myID)) != ERROR_ok)
{
TSLogging::Error("(ts3plugin_onClientMoveEvent)",s erverConnectionHandlerID,error);
return;
}

The question is, how does getClientId look like in the wrapper lib in the end? Does it internally include the connect status check solely for this quirk? In case of with or without, does it return something? Does it throw? Both, depending on the problem?

I am somewhat in doubt that exceptions can/should completely replace returning error codes here due to the non-rarity of some errors. That leads to having both, which leads to having to make a number of informed decisions around the nature of things being a simple result, a common non-severe error that either the lib or the user should preferably deal with locally, or an exception-worthy error.

That said, lot's of strong points for exceptions can be seen at https://isocpp.org/wiki/faq/exceptions#why-exceptions (cpp, but arguments somewhat transferable I guess).

[numma_cway]

Personal concerns with using exceptions (exclusively):
Not everything that isn't ERROR_ok is exceptional.
Looking at ERROR_ok_noupdate, ERROR_not_connected and whatnot.

Those don't exist in query.

[thorwe]

The concerns, or better said the facets of a single concern, were merely meant to be in the context of the original post's C# plugin api wrapper task - it wasn't meant to be a generalization.
Still even there, I wouldn't claim exceptions to be the worst idea ever. Both are better than using a global variable, that's a thing I'd be confident to say
How a throughout usage of exceptions for all kinds of not-ok is perceived again is probably a bit of a language and/or school of thought thing. Language wise C# programmers I'd assume to be relatively comfortable with such, given they're the target audience there'd be a plus.

[Chris]

Those don't exist in query.

You're forgetting that this is about a general plugin wrapper and NOT a query wrapper. As such most of your points are kind of moot / off the point I guess.

[Birdboat]

Right now, all the API functions are going to log the error. But if need be I can change it in the future.
I've made a class called TSReturnValue, it's just a mutable Tuple:

public class TSReturnValue<T1>
{
    public Error Error { get; set; }
    public T1 Value { get; set; }
}

public class TSReturnValue<T1, T2>
{
    public Error Error { get; set; }
    public T1 Value1 { get; set; }
    public T2 Value2 { get; set; }
}

All of the API functions that have a separate value will return these classes. So it practice it would look like:

var result = Plugin.Api.GetClientLibVersion();
if (result.Error == Error.ok) Console.WriteLine(result.Value);
else ; // Handle the error here

Let me know what you guys think