Over-Abstraction? A debate.
I've been having an argument with a co-worker of mine about the KeyboardHook code I recently published here.
His claim - by exposing a KeyboardHook object and letting users add Filters for specific keystrokes, I am exposing too much of my inner implementation that shouldn't interest the user of my library. Properly abstracted and refactored, the user should treat each KeyboardHook object as a specific keystroke-capture object, not have to deal with two different objects here. Various implementation details like explicitly calling Install() should be implicitly called whenever the event is hooked up to an event handler, rather than being left to the innocent caller.
While my code uses this:
KeyboardHook hook = new KeyboardHook();
hook.AddFilter(Keys.F, true, false, true);
hook.AddFilter(Keys.F6, false, true, false);
hook.KeyPressed +=new KeyboardHookEventHandler(hook_KeyPressed);
hook.Install();
He believes it should be more like this:
KeyboardHook firstHook = KeyboardHook.Create(Keys.F, true, false, true);
KeyboardHook secondHook = KeyboardHook.Create(Keys.F6, false, true, false);
firstHook.KeyPressed +=new KeyboardHookEventHandler(firstHook_KeyPressed);
secondHook.KeyPressed +=new KeyboardHookEventHandler(secondHook_KeyPressed);
My counter-claim is that while this may be more abstract and refactored, it may be taking it a bit too far. A Windows hook is an actual operating system object, not just a logical construct I create in my code, and there can be serious performance implications for defining one of them for each keystroke rather than defining just one generic hook for all strokes. Furthermore, I want the users of my library to be aware of this difference, rather than hiding this information. I want them to be aware of the implications of having a hook installed, and allow them to install and uninstall it selectively.
His counter-counter-claim was that I can implement all my static methods to hide the actual system hook and have all my KeyboardHook objects actually receive their events from that one OS object. I can have a reference counting mechanism set up to allow me to disable individual hooks, and only uninstall the inner hook when all stroke-captures are disabled.
My prompt reply was that this might, indeed, produce rather clean and elegant code. However, it would also involve rewriting most of the existing code for this library for a relatively small and theoretical gain for the library user, and I'll be damned if I go and do that any time soon, and he's welcome to do it himself if he-thinks-he-knows-so-much.
The conversation generally deteriorated from that point onwards.
So, anyone have any comments on the current design or the proposed changes, the benefits and shortcomings of both?