I'm evaluating a library whose public API currently looks like this:
libengine.h
/* Handle, used for all APIs */ typedef size_t enh; /* Create new engine instance; result returned in handle */ int en_open(int mode, enh *handle); /* Start an engine */ int en_start(enh handle); /* Add a new hook to the engine; hook handle returned in h2 */ int en_add_hook(enh handle, int hooknum, enh *h2);
Note that enh
is a generic handle, used as a handle to several different datatypes (engines and hooks).
Internally, most of these APIs of course cast the "handle" to an internal structure which they've malloc
'd:
engine.c
struct engine { // ... implementation details ... }; int en_open(int mode, *enh handle) { struct engine *en; en = malloc(sizeof(*en)); if (!en) return -1; // ...initialization... *handle = (enh)en; return 0; } int en_start(enh handle) { struct engine *en = (struct engine*)handle; return en->start(en); }
Personally, I hate hiding things behind typedef
s, especially when it compromises type safety. (Given an enh
, how do I know to what it's actually referring?)
So I submitted a pull request, suggesting the following API change (after modifying the entire library to conform):
libengine.h
struct engine; /* Forward declaration */
typedef size_t hook_h; /* Still a handle, for other reasons */
/* Create new engine instance, result returned in en */
int en_open(int mode, struct engine **en);
/* Start an engine */
int en_start(struct engine *en);
/* Add a new hook to the engine; hook handle returned in hh */
int en_add_hook(struct engine *en, int hooknum, hook_h *hh);
Of course, this makes the internal API implementations look a lot better, eliminating casts, and maintaining type safety to/from the consumer's perspective.
libengine.c
struct engine
{
// ... implementation details ...
};
int en_open(int mode, struct engine **en)
{
struct engine *_e;
_e = malloc(sizeof(*_e));
if (!_e)
return -1;
// ...initialization...
*en = _e;
return 0;
}
int en_start(struct engine *en)
{
return en->start(en);
}
I prefer this for the following reasons:
- Added type safety
- Improved clarity of types and their purpose
- Removed casts and
typedef
s - It follows the recommended pattern for opaque types in C
However, the owner of the project balked at the pull request (paraphrased):
Personally I don't like the idea of exposing the
struct engine
. I still think the current way is cleaner & more friendly.Initially I used another data type for hook handle, but then decided to switch to use
enh
, so all kind of handles share the same data type to keep it simple. If this is confusing, we can certainly use another data type.Let's see what others think about this PR.
This library is currently in a private beta stage, so there isn't much consumer code to worry about (yet). Also, I've obfuscated the names a bit.
How is an opaque handle better than a named, opaque struct?
Note: I asked this question at Code Review, where it was closed.