{"id":4911,"date":"2015-10-14T13:00:39","date_gmt":"2015-10-14T13:00:39","guid":{"rendered":"http:\/\/writeasync.net\/?p=4911"},"modified":"2015-10-14T04:34:20","modified_gmt":"2015-10-14T04:34:20","slug":"the-testable-adapter-anti-pattern","status":"publish","type":"post","link":"http:\/\/writeasync.net\/?p=4911","title":{"rendered":"The testable adapter anti-pattern"},"content":{"rendered":"

Let’s say you’re writing a module to deal with Windows optional features<\/a>. Following the guidance of the Ports\/Adapters\/Simulators<\/a> design approach, you come up with a port IFeatures<\/code> and an adapter WindowsFeatures<\/code>. “I must make my adapter testable!” you exclaim, and after a series of coding and testing steps, you come up with something like this:<\/p>\n

\r\ninternal sealed class Program\r\n{\r\n    private static void Main(string[] args)\r\n    {\r\n        IFeatures features = CreateFeatures();\r\n        \/\/ ...\r\n    }\r\n\r\n    private static IFeatures CreateFeatures()\r\n    {\r\n        List<string> installed = new List<string>();\r\n\r\n        ManagementScope scope = new ManagementScope(@"\\\\.\\ROOT\\cimv2");\r\n        ObjectQuery query = new ObjectQuery("SELECT * FROM Win32_OptionalFeature");\r\n        using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(scope, query))\r\n        using (ManagementObjectCollection results = searcher.Get())\r\n        {\r\n            foreach (ManagementObject result in results)\r\n            {\r\n                if ((uint)result["InstallState"] == 1)\r\n                {\r\n                    installed.Add((string)result["Name"]);\r\n                }\r\n            }\r\n        }\r\n\r\n        return new WindowsFeatures(installed, new WindowsFeatureFactory());\r\n    }\r\n\r\n    private sealed class WindowsFeatureFactory : IFeatureFactory\r\n    {\r\n        public IFeature Create(string name)\r\n        {\r\n            return new WindowsFeature(name);\r\n        }\r\n\r\n        private sealed class WindowsFeature : IFeature\r\n        {\r\n            private readonly string name;\r\n\r\n            public WindowsFeature(string name)\r\n            {\r\n                this.name = name;\r\n            }\r\n\r\n            public void Enable()\r\n            {\r\n                \/\/ ... implementation code goes here , probably using\r\n                \/\/ https:\/\/msdn.microsoft.com\/en-us\/library\/windows\/desktop\/hh825834.aspx ...\r\n            }\r\n\r\n            public void Disable()\r\n            {\r\n                \/\/ ... more implementation code ...\r\n            }\r\n        }\r\n    }\r\n}\r\n<\/pre>\n
The humble adapter ends up this way:<\/p>\n
\r\npublic sealed class WindowsFeatures : IFeatures\r\n{\r\n    private readonly HashSet<string> enabled;\r\n    private readonly IFeatureFactory featureFactory;\r\n\r\n    public WindowsFeatures(IEnumerable<string> enabled, IFeatureFactory featureFactory)\r\n    {\r\n        this.enabled = new HashSet<string>(enabled);\r\n        this.featureFactory = featureFactory;\r\n    }\r\n\r\n    public void Enable(string name)\r\n    {\r\n        if (!this.enabled.Contains(name))\r\n        {\r\n            this.enabled.Add(name);\r\n            IFeature feature = featureFactory.Create(name);\r\n            feature.Enable();\r\n        }\r\n    }\r\n\r\n    public void Disable(string name)\r\n    {\r\n        if (this.enabled.Contains(name))\r\n        {\r\n            this.enabled.Remove(name);\r\n            IFeature feature = featureFactory.Create(name);\r\n            feature.Disable();\r\n        }\r\n    }\r\n}\r\n<\/pre>\n
And of course, some unit tests to show the logic is sound:<\/p>\n
\r\n[TestMethod]\r\npublic void ShouldEnableTheFeatureIfNotEnabled()\r\n{\r\n    StubFeatureFactory featureFactory = new StubFeatureFactory();\r\n    WindowsFeatures features = new WindowsFeatures(new string[0], featureFactory);\r\n\r\n    features.Enable("NewFeature");\r\n\r\n    Assert.AreEqual(1, featureFactory.Features.Count);\r\n    Assert.IsTrue(featureFactory.Features.ContainsKey("NewFeature"));\r\n    Assert.AreEqual(1, featureFactory.Features["NewFeature"].EnableCount);\r\n}\r\n\r\n[TestMethod]\r\npublic void ShouldNotEnableTheFeatureIfAlreadyEnabled()\r\n{\r\n    StubFeatureFactory featureFactory = new StubFeatureFactory();\r\n    WindowsFeatures features = new WindowsFeatures(new string[] { "OldFeature" }, featureFactory);\r\n\r\n    features.Enable("OldFeature");\r\n\r\n    Assert.AreEqual(0, featureFactory.Features.Count);\r\n}\r\n\r\n[TestMethod]\r\npublic void ShouldDisableTheFeatureIfEnabled()\r\n{\r\n    StubFeatureFactory featureFactory = new StubFeatureFactory();\r\n    WindowsFeatures features = new WindowsFeatures(new string[] { "OldFeature" }, featureFactory);\r\n\r\n    features.Disable("OldFeature");\r\n\r\n    Assert.AreEqual(1, featureFactory.Features.Count);\r\n    Assert.IsTrue(featureFactory.Features.ContainsKey("OldFeature"));\r\n    Assert.AreEqual(1, featureFactory.Features["OldFeature"].DisableCount);\r\n}\r\n\r\n[TestMethod]\r\npublic void ShouldNotDisableTheFeatureIfAlreadyDisabled()\r\n{\r\n    StubFeatureFactory featureFactory = new StubFeatureFactory();\r\n    WindowsFeatures features = new WindowsFeatures(new string[0], featureFactory);\r\n\r\n    features.Disable("NewFeature");\r\n\r\n    Assert.AreEqual(0, featureFactory.Features.Count);\r\n}\r\n<\/pre>\n
It’s testable and makes use of some nice dependency inversion<\/a>. So what’s the problem?<\/p>\n
Well, unfortunately, this adapter contains absolutely zero code relevant to Windows features, despite its promise. That’s the catch — in order to make it “testable,” the essence of what made it an adapter (i.e. management of external dependencies) has vanished. Even worse, two new interfaces of indirection<\/a> were introduced solely to serve the tests, which themselves are essentially shallow demonstrations of an idempotence guarantee.<\/p>\n
Making your adapters testable might seem like sound design, but you must resist the urge lest you end up with confused object models like the above. Instead, focus on making the adapter as thin and boring as possible so that you have no need to test it (or at least, not extensively<\/em> beyond basic contract tests<\/a>).<\/p>\n
For instance, if the goal is to avoid calling potentially expensive code if you know the action is unnecessary, why not make smarter data structures which can clearly record the design intent instead? Here is a set which can call a post-action after successfully adding or removing an item:<\/p>\n
\r\npublic static class SetExtensions\r\n{\r\n    public static void Add<T>(this ISet<T> set, T item, Action ifAdded)\r\n    {\r\n        if (set.Add(item))\r\n        {\r\n            ifAdded();\r\n        }\r\n    }\r\n\r\n    public static void Remove<T>(this ISet<T> set, T item, Action ifRemoved)\r\n    {\r\n        if (set.Remove(item))\r\n        {\r\n            ifRemoved();\r\n        }\r\n    }\r\n}\r\n<\/pre>\n
The tests can now operate directly at this level of abstraction instead of faking that they have anything to with feature installs:<\/p>\n
\r\n[TestMethod]\r\npublic void ShouldRunActionAfterAdd()\r\n{\r\n    ISet<string> set = new HashSet<string>();\r\n    bool ran = false;\r\n\r\n    set.Add("newItem", () => ran = true);\r\n\r\n    Assert.IsTrue(set.Contains("newItem"));\r\n    Assert.IsTrue(ran);\r\n}\r\n\r\n[TestMethod]\r\npublic void ShouldSkipActionIfAlreadyAdded()\r\n{\r\n    ISet<string> set = new HashSet<string>(new string[] { "existingItem" });\r\n    bool ran = false;\r\n\r\n    set.Add("existingItem", () => ran = true);\r\n\r\n    Assert.IsFalse(ran);\r\n}\r\n\r\n[TestMethod]\r\npublic void ShouldRunActionAfterRemove()\r\n{\r\n    ISet<string> set = new HashSet<string>(new string[] { "existingItem" });\r\n    bool ran = false;\r\n\r\n    set.Remove("existingItem", () => ran = true);\r\n\r\n    Assert.IsFalse(set.Contains("existingItem"));\r\n    Assert.IsTrue(ran);\r\n}\r\n\r\n[TestMethod]\r\npublic void ShouldSkipActionIfAlreadyRemoved()\r\n{\r\n    ISet<string> set = new HashSet<string>();\r\n    bool ran = false;\r\n\r\n    set.Remove("nonexistentItem", () => ran = true);\r\n\r\n    Assert.IsFalse(ran);\r\n}\r\n<\/pre>\n
And the WindowsFeature<\/code> code can be one step closer to trivial:<\/p>\n
\r\npublic void Enable(string name)\r\n{\r\n    this.enabled.Add(name, () => this.DoEnable(name));\r\n}\r\n\r\npublic void Disable(string name)\r\n{\r\n    this.enabled.Remove(name, () => this.DoDisable(name));\r\n}\r\n\r\nprivate void DoEnable(string name)\r\n{\r\n    IFeature feature = this.featureFactory.Create(name);\r\n    feature.Enable();\r\n}\r\n\r\nprivate void DoDisable(string name)\r\n{\r\n    IFeature feature = this.featureFactory.Create(name);\r\n    feature.Disable();\r\n}\r\n<\/pre>\n
Testability can be a helpful attribute but should not be and end in itself. Especially when dealing with the edges of your system, look for ways to get closer to the correct by construction<\/a> ideal.<\/p>\n","protected":false},"excerpt":{"rendered":"
Let’s say you’re writing a module to deal with Windows optional features. Following the guidance of the Ports\/Adapters\/Simulators design approach, you come up with a port IFeatures and an adapter WindowsFeatures. “I must make my adapter testable!” you exclaim, and… <\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[91,41],"tags":[],"class_list":["post-4911","post","type-post","status-publish","format-standard","hentry","category-design","category-tdd"],"_links":{"self":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/4911","targetHints":{"allow":["GET"]}}],"collection":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=4911"}],"version-history":[{"count":0,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/4911\/revisions"}],"wp:attachment":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=4911"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=4911"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=4911"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}