Still working on cleaning up some legacy ASP.NET code. Here’s where we are:
- Part 1: Define problem and demonstrate IFileSystem basic version
- Part 2: Spike solution to support saving files in IFileSystem that works in both Amazon S3 and the Windows file system
- Part 3: Initial refactoring via TDD of big ugly method
Now it’s time to take the big step of pulling the main ugly method guts out into its own object. Since the main purpose of the method, GetImageOrFlashData(), is to store a file that has been uploaded, I’m thinking at the moment that the name for the class who will take on this responsibility is going to be CreativeFileStore (a creative is a noun in this context; a creative file is a file representing a creative, not a file that uses its left brain). What should the CreativeFileStore’s interface look like?
- Accepts an IFileSystem in its constructor
- Implements a WriteFile() method with required parameters for the existing method to use it to replace its current functionality.
Since we know our current implementation relies on the System.IO, it’s simple enough to have the class use the WindowsFileSystem by default, resulting in a basic structure like so:
We also need to update IFileSystem (and its implementation) to support writing files based on the code we came up with during our spike solution (in part 2). Here’s the new IFileSystem interface:
And here’s the WindowsFileSystem implementation of WriteBinaryFile:
We determined in the spike solution that the windows file system would need to know an upload path, so we’ve added a property StorageFolderPath to represent this. It will be up to whatever part of the system creates my implementation of IFileSystem to ensure that any additional properties like this one are properly set. In any event, if a full path is given to WriteBinaryFile, the lack of a StorageFolderPath shouldn’t cause it to fail. That sounds like an assumption – let’s verify with a test, shall we?
Pull out the Path.Combine into a separate method (extract method) which in our case we’ll make protected since nothing outside this class should need it.
Then we can test it easily enough by simply subclassing, like so:
These pass, so it sounds like we got this right. Now back to CreativeFileStore, which needs a method for writing files. This method replaces the goo that was in GetImageOrFlashData(), which when we’re done should look pretty much like this:
In the original implementation, we called into an upload control’s PostedFile property to do SaveAs(filePath). We can now replace this with a call to our file system’s WriteBinaryFile() method. The resulting method looks something like this (doesn’t quite compile):
Now we need to refactor this library call so that it doesn’t depend on an HttpPostedFile, then add some tests, and we’re done (finally). What this code does is determine the dimensions of the uploaded image and verify they’re what’s expected, throwing an exception if not. To get this to compile for now, we can replace the call with the same thing that we used for .swf files:
In this part, we extracted an ugly method into its own class and along the way wrote some tests and added some properties and methods to our IFileSystem and WindowsFileSystem components. The end result is a testable CreativeFileStore (we haven’t written enough tests yet – I decided this was getting long enough so they’ll come in the next part) that (currently) is missing a little functionality it had before (verifying image dimensions) but which we’ll work on restoring in the next step.