Information Hiding is a critical, but tricky principle to get familiar with. My work on the Ionide Test Explorer presented some good examples of refactoring to limit scope (Information Hiding). I’ll try to recreate them as worked examples.
Up front, I want to be clear that I’m not trying to bash the code that came before. The previous author was exploring many new concepts while trying hash out a viable product, graciously giving their time to help the community. They did a good job keeping the explorer contained and creating reusable code analysis. Many of these methods I’ll refactor likely started small and grew past their original scope, as methods do. Code and our own understanding change, and that’s why refactoring is a continual effort.
Exploring The Existing Code
I set out to make some underlying changes to the Ionide test explorer (specifically, to use test results as the source of truth instead of code analysis). But first I needed to understand the existing explorer.
After some poking, I found that the whole test explorer was contained in components/TestExplorer.fs. Great, a fairly limited scope.
A bit more poking, and I found the method that VS Code calls when the user requests a test run,
runHandler (show below).
I can quickly see from method calls that
runHandler finds projects, builds projects, and runs test projects.
The next bit is long and hard to decipher at a glance. After a bit of reading. I can tell that this is setting test outcome statuses in the UI.
Clarifying Intent with Methods
Ok, so let’s change the code to make this newfound knowledge more immediately apparent. We’re looking for a consistent level of abstraction in
runHandler and a separable UI updating method that’s as independent as possible.
Let’s start by moving the status setting code to it’s own method and seeing what it requires to compile.
This reveals the UI state is controlled by the TestRun, which requires an instance of a TestItem (the visual explorer item instance) to set a status. We also need the test outcome (i.e. pass/fail) along with potential error messages and such.
Unnecessary Coupling Revealed
Refactoring the map from test outcomes to visual state into its own method reveals some unnecessary coupling.
Specifically, our TestItem instance is a member of TestResult. That seems like a violation of information hiding. TestItem is a UI-specific type. I might want to know the outcome of a test without needing to know about the UI element it ties back to. For example, if I just want to log the test’s name and result in a console (which I later did). Dragging the UI element along adds unnecessary complexity.
Further, I checked how TestResults are constructed and, sure enough, including a TestItem was dragging UI knowledge down the call chain. Specifically into the test result parsing code under
Extracting test results from a TRX file shouldn’t require knowledge of the UI, that’s a violation of Information Hiding. Parsing a test result file is an independent concern. In fact, it’s not hard to imagine it as a stand-alone library that many applications would use, let alone other uses in our own project. But, the TestItem coupling prevents the TRX parser from being reused at all and makes the code more difficult to understand.
Finding a Gradual Refactor
Ok. So we can see this is moving in a direction where TestItem unbundles from TestResult, but that could be a wide-reaching change that forces us into a snowball of changes. We want small and complete commits that don’t force wide-reaching changes, but that eventually add up to the desired decoupling.
A good strategy for this is to push the concern up another level. In our case, our new visual state mapping method can pretend like TestResult doesn’t contain a TestItem and require the TestItem to be passed separately. This is one way to apply the Strangler Fig refactoring pattern.
Now we’re back to the level we started with, but
runHandler is more concise and readable.
This change is pretty safe. It didn’t change any behavior and it’s small enough in scope that it’s easy to understand and review. Yet it meaningfully improved the understandability of the code and stepped us closer to some wide-scale refactors.
We also identified that
runProject has some mixed responsibilities, and some better information hiding could potentially create more reusable components.
I’ll dig into that next post.
Full Code Links
Note, the full Ionide test explorer work is available if you want to explore in more depth. The the final code diff is a bit useless because the extensive changes. It might be better to read the original code then the updated version.