You know you’ve found a bad C# API when…

I’ve been evaluating a certain, unnamed 3rd party vendor.  They have a programming library API for integrating their product into OEM solutions.

I purposefully don’t want to point fingers, or name specific names, but this API is one of the worst C# API’s I’ve ever seen.  I don’t understand why they even bother providing a C# API and samples…

NOTE: I’m going to rename some of the routines and names in order to not be rude to this vendor.  I may end up using their product (since they have better hardware than software), but just couldn’t believe what I was seeing.  I don’t know how some developers sleep at night when they write C# like this.

The API consists of a single C# file, using P/Invoke to interface with a native API.  The C# API they provide is 106 lines long.  Of the 106 lines, nearly all of it consists of lines like the following:

// function keywords
const ushort VENDORNAME_FIND       = 1;
const ushort VENDORNAME_FIND_NEXT  = 2;
const ushort VENDORNAME_LOGON      = 3;
const ushort VENDORNAME_LOGOFF     = 4;
// Repeat for code 5 - 42!!!
 
// Error code
const ushort SUCCESS               = 0;
const ushort ERROR_NO_DEVICE       = 3;
const ushort ERROR_NO_DEVICE       = 4;
const ushort ERROR_BAD_PASSWORD    = 5;
const ushort ERROR_UNKNOWN_COMMAND = 9;
// Repeat for random codes up to 41
// Then add some random codes defined in hex instead

This is bad enough to start with – obviously, this is a straight port of their C API.  No care was taken to treat anything the way that it would be done in C# or .NET – everything is horrible error codes and keywords in constant values.  That alone is worthy of a rant, but it gets worse…

How can it be worse than this for a C# developer, you may ask?

The API has exactly 3 methods defined via P/Invoke.  Two of the three methods are P/Invoking into the MSVCRT.dll, and providing routines that exist in the BCL!

[DllImport("MSVCRT")]
public static extern uint time(ref uint timer);
[DllImport("MSVCRT")]
public static extern void srand(uint seed);

What good does this do?  Why not just use DateTime.Now and System.Random, like every sane C# program?  I’m sure these routines are added here so they could port their C sample code directly in C#, without needing to know C#.

Finally, they have a single method defining their entire API –

[DllImport("VendorName")]
public static extern ushort VendorNameApiMethod(
        ushort function, ref ushort handle,
        ref uint lp1, ref uint lp2, ref ushort p1, ref ushort p2,
        ref ushort p3, ref ushort p4, Byte[] buffer);

This single method implements the entire API for this library!

The library should actually be comprised of 42 methods (remember our function keywords?), but instead, they put it all into a single routine.  The usage of this function completely changes depending on which function you pass into the first parameter – for example, if you pass VENDORNAME_FIND, it will set “handle” to be a handle.  If you set VENDORNAME_DOSOMEWORK, you need to pass in the appropriate handle (instead of returning it), and it will set values in one of the random uint, ushort, or in the Byte[] buffer.  Each of the 42 commands performs a different function, but none of it is commented.

This API is bad enough in C – at least when I’m writing C, I expect cumbersome APIs to work without classes, encapsulation, etc.  But when I’m writing C++, I’d expect this to be wrapped into a reasonable class.  When I write in C#, I expect it to use exceptions for error conditions, to have a clean class with methods, and to hopefully follow the basic design guidelines for .NET.

To all developers out there – please, please, I plead you, do not allow this type of code to be introduced into the .NET world.  It just makes your company look like they do not know what they are doing.  Even this horrible C API could be wrapped into a nice class in a few hours.  Take the time to think in C#.