IS4Code / PawnPlus

A SA-MP plugin enhancing the capabilities of the Pawn programming language
MIT License
103 stars 17 forks source link

Return codes or AMX errors? #13

Closed IS4Code closed 5 years ago

IS4Code commented 5 years ago

The title says it all. The issue is whether functions that are given invalid parameters should raise an error or return a special error code.

At the moment, AMX errors are raised only when a function is called with an invalid format of arguments. Unpaired keys and values for map construction, argument format strings with invalid number of arguments, or calling a function with less arguments provided than required.

In my opinion, raising an AMX error is appropriate on these occasions, since the error comes from incorrect explicit usage of the function, and can be understood and fixed by working only with the line of the call. Other natives return special values signaling failure whenever possible (usually 0 or -1).

However, based on my experience, error codes are rarely checked by programmers, and therefore aren't a reliable way to inform the programmer in the majority of cases.

Imagine this function:

List:GetListOfSomething()
{
    new List:l; // list_new() is forgotten
    list_add(l, 3);
    list_add(l, 1);
    list_add(l, 4);
    return l;
}

Adding to a nonexistent list is certainly a failure, and thus list_add returns -1, signalling an error. However, one does not usually catch this error, and most importantly, one cannot recover from this error in any sensible way.

Imagine the function was supposed to return a list of houses the player has bought. The mistake can be easily missed, and with lack of proper testing, it can erase the houses of all players, before being discovered.

In my opinion, when this kind of error happens, the code should "fail fast", which is certainly better than continuing execution in an unexpected and undesired state, since most likely, the error will be completely ignored, cascading into other errors further in the code.

So, my proposal is to get rid of return codes, and raise AMX errors for errors that satisfy all of the following:

Perhaps there errors can be better described by what Eric Lippert calls boneheaded exceptions.

In all these cases, an AMX error would be raised with the code representing the invalid operation, and an informative message would be printed. Hopefully, this would be caught during the testing of any script.

If such an error gets into production, PawnPlus already offers ways to catch it, if it is needed. There is task_bind and amx_fork that can get the AMX error code, so the programmer can decide whether to shut down the server, or simply print an error message to the player (if it is not that fatal).

Since this is once again a breaking change, albeit what I find a useful one, I want first to discuss it with you, to share thoughts and opinions.

IS4Code commented 5 years ago

It's left to the debate whether map_get should raise an error or not if the key is not present in the map.