Today’s API design fail case study is Iconv. It is a library designed to convert text from one encoding to another. The basic API is very simple as it has only three function calls. Unfortunately two of them are wrong.
Let’s start with the initialisation. It looks like this:
iconv_t iconv_open(const char *tocode, const char *fromcode);
Having the tocode argument before the fromcode argument is wrong. It goes against the natural ordering that people have of the world. You convert from something to something and not to something from something. If you don’t believe me, go back and read the second sentence of this post. Notice how it was completely natural to you and should you try to change the word order in your mind, it will seem contrived and weird.
But let’s give this the benefit of the doubt. Maybe there is a good reason for having the order like this. Suppose the library was meant to be used only by people writing RPN calculators in Intel syntax assembly using the Curses graphics library. With that in mind, let’s move on to the second function as it is described in the documentation.
size_t iconv (iconv_t cd, const char* * inbuf, size_t * inbytesleft,
char* * outbuf, size_t * outbytesleft);
In this function the order is the opposite: source comes before target. Having the order backwards is bad, but having an inconsistent API such as this is inexcusable.
But wait, there is more!
If you look at the actual installed header file, this is not the API it actually provides. The second argument is not const in the implementation. So either you strdup your input string to keep it safe or cast away your const and hope/pray that the implementation does not fiddle around with it.
The API function is also needlessly complex, taking pointers to pointers and so on. This makes the common case of I have this string here and I want to convert it to this other string here terribly convoluted. It causes totally reasonable code like this to break.
char *str = read_from_file_or_somewhere();
iconv(i, &str, size_str, &outbuf, size_outbuf);
Iconv will change where str points to and if it was your only pointer to the data (which is very common) you have just lost access to it. To get around this you have to instantiate a new dummy pointer variable and pass that to iconv. If you don’t and try to use the mutilated pointers to, say, deallocate a temporary buffer you get interesting and magical crashes.
Passing the conversion types to iconv_open as strings is also tedious. You can never tell if your converter will work or not. If it fails, Iconv will not tell you why. Maybe you have a typo. Maybe this encoding has magically disappeared in this version. For this reason the encoding types should be declared in an enum. If there are very rare encodings that don’t get built on all platforms, there should be a function to query their existence.
A better API for iconv would take the current conversion function and rename it to iconv_advanced or something. The basic iconv function (the one 95% of people use 95% of the time) should look something like this:
int iconv(encoding fromEncoding, encoding toEncoding,
errorBehaviour eb,
const char *source, size_t sourceSize,
char *target, size_t targetSize);
ErrorBehaviour tells what to do when encountering errors (ignore, stop, etc). The return value could be total number of characters converted or some kind of an error code. Alternatively it could allocate the target buffer by itself, possibly with a user defined allocator function.
The downside of this function is that it takes 7 arguments, which is a bit too much. The first three could be stored in an iconv_t type for clarity.
Read more
Latest Official Posts
