Refactoring File System Access

Refactoring File System Access

Years ago, I was trying to test an application I’d written, but couldn’t figure out how to remove a dependency it had on the file system. At the time, I was familiar with unit testing, and had bought into its value both as a developer and as a business owner (this was software that was central to my business at that time), but I was stymied by some code that looked something like this:

Now, I could pretty easily write some unit tests for the CreateFileNameForPublisher method if I made it public or moved it into its own type, assuming it didn’t also work directly with the file system (it did). But trying to write unit tests for the SaveBanner method was proving impossible in its current form. This is another example of an insidious dependency, one of those things that makes writing tests difficult and adds coupling between our business logic and our infrastructure code.

Leaving aside direct testing of CreateFileNameForPublisher, let’s look at how we can refactor SaveBanner to allow its behavior to be tested. We can test that the file creation logic actually works by either running it manually (and trusting it will continue working), or creating an integration test for it. The behavior of SaveBanner, though, should be at a higher level of abstraction. What we want to test is that it performs these operations:

  • Call a method to get a new unique filename for the publisher
  • Save the file using the filename retrieved
  • Call a method to save a new record with the publisherid and the file path used

The first step in modifying this code to be unit testable is to create an abstraction to replace the file system operations being performed, and then copy the low level file code into an implementation of the abstraction. Something like this will work:

Now it’s a simple matter to follow the Explicit Dependencies Principle and request an instance of IFileSystem in our BannerService. At this point we can also update the FileStream parameter to simply be a Stream, too.

Now we’ve fixed the main problem with testing SaveBanner, which was its direct dependency on the file system. However, there are still two more dependencies. The CreateFileNameForPublisher method *also* needs to talk to the file system, and the record saving part will of course use some data access technology. I’ll leave the last bit as an exercise for the reader since I haven’t even shown its implementation here (hint: consider a Repository). For the private method call, though, we have a couple of options.

We could use a mocking tool that will mock private methods on concrete types.

We could create another interface and move the filename logic into a new type. This might make sense, since our BannerService is likely violating the Single Responsibility Principle and may be exhibiting the Iceberg Class code smell, with its private functionality. However, I’ve already shown this technique for IFileSystem, so let’s consider another approach.

We could modify BannerService so that we can control the behavior of CreateFileNameForPublisher. This does require one small change to the system under test (SUT): we need to add the virtual keyword and mark the private method as protected:

This should be a safe refactor, since we know nothing was depending on it before (because it was private).

Now we can create a test version of BannerService that inherits from BannerService and exposes this method in a way that we can alter:

At this point, you can write a test that uses the TestBannerService by instantiating it with a known format string, which you can then verify is used in the call to CreateFileFromStream:

Having an abstraction for the file system in this case wasn’t just good for testing. It also made it much easier to move to using a CDN for the files in question. Instead of having to change many locations that worked directly with files (violating the Open Closed Principle), adding support for CDNs simply involved creating a new implementation of IFileSystem for each CDN provider.

  • Илья Марголин

    Cool explanation of concept. In real world please use something like http://www.nuget.org/packages/System.IO.Abstractions/

    • Came by to say the same. See https://github.com/tathamoddie/System.IO.Abstractions for docs.
      One of the main advantages of this over writing it yourself is the behavior of the provided mocks and wrappers is consistent with the expectations of a real file system.

    • ardalis

      That’s a good resource. However, unless I need all of its functionality, I’m more likely to just write my own smaller interface that is tailored to my client code’s needs. This follows the Interface Segregation Principle (http://deviq.com/interface-segregation-principle/), preventing my types from relying on more than they need. This of course assumes I’m writing something from scratch…

      If I have a legacy codebase that’s making heavy use of the file system, then using a package like this that mirrors the interface of System.IO’s classes makes a ton of sense and should save a bunch of time if you’re trying to refactor and get the code under tests.

      Thanks for sharing!