{"id":1251,"date":"2014-01-20T13:00:19","date_gmt":"2014-01-20T13:00:19","guid":{"rendered":"http:\/\/writeasync.net\/?p=1251"},"modified":"2014-01-15T08:34:04","modified_gmt":"2014-01-15T08:34:04","slug":"async-process-functionality-take-2","status":"publish","type":"post","link":"http:\/\/writeasync.net\/?p=1251","title":{"rendered":"Async Process functionality – take 2"},"content":{"rendered":"

In my previous post<\/a>, I showed a simple way to add a few Task-based async methods for dealing with System.Diagnostics.Process<\/code><\/a>. The code I showed, while simple, has some flaws:<\/p>\n

    \n
  1. It has low cohesion.<\/strong> In violation of the single responsibility principle<\/a>, the ProcessEx<\/code> class is doing at least three things: acting as a factory for starting processes, watching for process exit events, and providing access to a more full-featured underlying Process<\/code> object. (The relatively weak “Ex<\/code>” name is a good clue that this class doesn’t have a clear purpose.)<\/li>\n
  2. The wait operation cannot be canceled.<\/strong> Perhaps a somewhat minor point, but it is a good idea for any “WaitXxx<\/em>Async<\/code>” API to support cancellation<\/a>.\n
  3. It is not testable.<\/strong> I can’t write “pure” unit tests<\/a> against a class that requires me to spawn a real, live external process and waste time waiting for a real, live exit condition.<\/li>\n<\/ol>\n

    The last point (testability) was what initially bothered me. I was able to convince myself of the correctness of the code via integration testing, but since TDD is primarily about design<\/a>, I couldn’t vouch for the quality of the implementation. And when I looked more closely, some of problems in the first point (cohesion) jumped out at me.<\/p>\n

    So… I dutifully tried again! You can see the evolution of what came to be known as ProcessExitWatcher<\/code> in the ProcessSample project on GitHub<\/a>.<\/p>\n

    I’ll highlight some interesting checkpoints of this effort.<\/p>\n

    In the beginning, I started with the same weak name (ProcessEx<\/code>) and a single unit test to demonstrate the constructor behavior<\/a> (subscribing to an Exited<\/code> event). At that point, I actually needed something to provide this event — hence my test stub<\/a> ProcessStub<\/code> implementing IProcess<\/code>.<\/p>\n

    A few steps later, I began to implement the WaitForExitAsync<\/code> functionality<\/a>. Eventually I got to the point where I knew there was a race condition which could possibly complete the exited task twice. However, instead of trying something crazy like resorting to a nondeterministic, multithreaded integration test, I wrote a unit test to expose the race<\/a> and solved it. In this case, I was able to extend my test stub and take advantage of code that I could inject via an event add<\/code> accessor<\/a> to sneakily raise the Exited<\/code> event.<\/p>\n

    Shortly after I added an Inner<\/code> property<\/a> I started to have some misgivings. Why should this class need to provide outside access to the full process object? After thinking on it a bit, I recognized that choosing the more specific class name ProcessExitWatcher<\/code> and making appropriate cascading naming changes<\/a> would provide better clarity.<\/p>\n

    Eventually I had to write the real<\/em> code that wired up my interface with a System.Diagnostics.Process<\/code>. This ended up being as mundane as you could imagine, a pass-through implementation<\/a>. That’s a good thing as it means no unit tests need to be written.<\/p>\n

    I wrote a few more unit tests but still sensed a code smell<\/a> in returning an IProcessExit<\/code> instance. The most obvious problem was the naming; a property called Inner<\/code> has encapsulation-breaking connotations. But the practical issue was that a caller would now have access to surface area that should not be needed or used (e.g. one could now set EnableRaisingEvents<\/code> to false<\/code> and mess things up). It was now apparent that the interface needed to be broken up (interface segregation<\/a>). Thus, I created IProcessExitEvents<\/code> and IProcessExitStatus<\/code><\/a> to separate concerns between the watcher and its clients. After all, the (conveniently read-only) status of the process is the only thing the caller should care about.<\/p>\n

    By the end of this, I had a lean and mean ProcessExitWatcher<\/code> and a clean, testable design I felt good about. Here it is in action:<\/p>\n

    \r\nint exitCode;\r\nDateTime exitTime;\r\n\r\nusing (Process process = await Task.Factory.StartNew(() => Process.Start("notepad.exe")))\r\nusing (ProcessExitWatcher watcher = new ProcessExitWatcher(new ProcessExit(process)))\r\n{\r\n    Console.WriteLine("Waiting for Notepad to exit...");\r\n    await watcher.WaitForExitAsync(token);\r\n    exitCode = watcher.Status.ExitCode;\r\n    exitTime = watcher.Status.ExitTime;\r\n}\r\n\r\nConsole.WriteLine("Done, exited with code {0} at {1}.", exitCode, exitTime);\r\n<\/pre>\n","protected":false},"excerpt":{"rendered":"
    In my previous post, I showed a simple way to add a few Task-based async methods for dealing with System.Diagnostics.Process. The code I showed, while simple, has some flaws: It has low cohesion. In violation of the single responsibility principle,… <\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[21,91,41],"tags":[],"class_list":["post-1251","post","type-post","status-publish","format-standard","hentry","category-async","category-design","category-tdd"],"_links":{"self":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/1251","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=1251"}],"version-history":[{"count":0,"href":"http:\/\/writeasync.net\/index.php?rest_route=\/wp\/v2\/posts\/1251\/revisions"}],"wp:attachment":[{"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=1251"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=1251"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/writeasync.net\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=1251"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}