When analyzing slimCODE.slimKEYS.Common.dll with FxCop, I get a few recommendations I'd like to share with you to get your feedback.
Members should not expose certain concrete types
FxCop does not like HotKeyHandlerProvider.LoadConfiguration and HotKeyHandler.LoadConfiguration to provide an XmlNode parameter for reading from it. Instead, it suggests passing an IXPathNavigable reference. I somehow feel this is a religious debate. I'm used to XmlNodes. But I know the XPath way seems popular. Comments?
While on the subject, how do you feel about the SaveConfiguration methods using a different scheme (i.e. an XmlWriter) for ease of use? Is this "inconsistency" bugging you?
It appears that an event handler is externally visible
FxCop thinks that HotKeyHandler.OnHotKeyPressed is an event handler. Though the pattern is very similar, it really needs to be a public method another class can call. I think my choice of name for this method is debatable. At first, "HotKeyPressed" could be better. On the other hand, one could say it does not have to be related to "an hotkey was pressed", but rather "hey, handler, do your thing!". So how about a name like "ProcessHotKey", with "key" and "modifiers" parameters, but without that useless "sender"?
As for the "VersionInfo" class not containing a private constructor, I already made that class static for the next version (great new thing in .NET 2.0). And the assembly will be marked "CLSCompliant" (sorry to fellow VB.NET developers).
The other recommendations I'm ignoring are:
- Use the word "Hotkey" instead of "HotKey" in type names, as "hotkey" is an accepted technical word.
- I like the capital K. HotKey it is!
- Do not pass types by reference in HotKeyManager.EditHandler.
- I don't see the point of complexifying such a simple method with a new type.
- Use a property instead of method IWindowsFormsEditor.GetPropertyEditor().
- This method does not expose the state of an object. It really "acquires" a property editor. Why play hide and seek with the "Get" verb when that's really what your method does?