Thursday, September 22, 2011

I loathe bool function parameters

What does HANDLE event = CreateEvent(NULL, TRUE, FALSE, NULL); do?
In this case bManualReset = TRUE and bInitialState = FALSE.

By combining patterns that I observed in the code of various co-workers and adding some of my own, I built the following pattern for making enums that can be used for parameters with any number of states, including bool.

namespace Win32
{
namespace Event
{
namespace Reset
{
enum type {
  // ensures that zero
  // initialized is invalid
  Invalid = 0,

  Manual,
  Auto,

  End,
  Begin = Manual
};

inline BOOL make(type from)
{
  switch (from) {
  case Manual:
    return TRUE;
  case Auto:
    return FALSE;
  default:
    terminate();
  };
}
}

namespace Value
{
enum type {
  // ensures that zero
  // initialized is invalid
  Invalid = 0,

  Set,
  Reset,

  End,
  Begin = Set
};

inline BOOL make(type from)
{
  switch (from) {
  case Set:
    return TRUE;
  case Reset:
    return FALSE;
  default:
    terminate();
  };
}
}
}
}

// now you can write this
// (the make functions are found via ADL)
namespace we = Win32::Event;
HANDLE event = nullptr;
event = CreateEvent(
    nullptr,
    make(we::Reset::Manual),
    make(we::Value::Reset),
    nullptr
    );

A remaining issue is that the compiler cannot enforce the type safety on the result of the make functions, so reversing the bManualReset and bInitialState parameters will compile without error and lie to readers of the code.

This issue does not plague functions that are defined to use the enum types. So, with a little more work:

namespace Win32
{
namespace Event
{
inline HANDLE Create(
  Reset::type reset,
  Value::type value,
  LPCWSTR name = nullptr
)
{
  return CreateEventW(
      nullptr,
      make(reset),
      make(value),
      name);
}

inline HANDLE Create(
  LPSECURITY_ATTRIBUTES security,
  Reset::type reset,
  Value::type value,
  LPCWSTR name = nullptr
)
{
  return CreateEventW(
      security,
      make(reset),
      make(value),
      name);
}
}
}

// now you can write this
namespace we = Win32::Event;
HANDLE event = nullptr;
event = we::Create(
    we::Reset::Manual,
    we::Value::Reset
    );

I general I try to avoid recreating named values and wrapping Api functions. Both the set of values and the valid inputs to the Api can change over time which requires me to chase the current definitions. Also, these change the surface of the Api, which creates an additional learning curve for callers. In this case where the api takes two BOOL parameters, I think the enums are valuable, but I am still somewhat ambivalent about the inline functions.

No comments:

Post a Comment