The way I approach a conversion is to look at any part of the system that permanently modifies state; files, database, external content. Once changed and re-read, has it changed for good? This is the first place to look to change it.
So the first thing you do, is find a place that modifies a source like this:
class MyXmlFileWriter
{
public bool WriteData(string fileName, string xmlText)
{
// TODO: Sort out exception handling
try
{
File.WriteAllText(fileName, xmlText);
return true;
}
catch(Exception ex)
{
return false;
}
}
}
Secondly, you write a unit test to make sure you aren't breaking the code while refactoring.
[TestClass]
class MyXmlWriterTests
{
[TestMethod]
public void WriteData_WithValidFileAndContent_ExpectTrue()
{
var target = new MyXmlFileWriter();
var filePath = Path.GetTempFile();
target.WriteData(filePath, "<Xml/>");
Assert.IsTrue(File.Exists(filePath));
}
// TODO: Check other cases
}
Next, Extract an Interface from the original class:
interface IFileWriter
{
bool WriteData(string location, string content);
}
class MyXmlFileWriter : IFileWriter
{
/* As before */
}
Re-run the tests and hope all is good. Keep the original test as it is checking your older implementation works.
Next write a fake implementation that does nothing. We only want to implement a very basic behaviour here.
// Put this class in the test suite, not the main project
class FakeFileWriter : IFileWriter
{
internal bool WriteDataCalled { get; private set; }
public bool WriteData(string file, string content)
{
this.WriteDataCalled = true;
return true;
}
}
Then unit test it...
class FakeFileWriterTests
{
private IFileWriter writer;
[TestInitialize()]
public void Initialize()
{
writer = new FakeFileWriter();
}
[TestMethod]
public void WriteData_WhenCalled_ExpectSuccess()
{
writer.WriteData(null,null);
Assert.IsTrue(writer.WriteDataCalled);
}
}
Now with it unit tested and refactored versions still working, we need to make sure that when injected, the calling class is using the interface, not the concrete version!
// Before
class FileRepository
{
public FileRepository() { }
public void Save( string content, string xml )
{
var writer = new MyXmlFileWriter();
writer.WriteData(content,xml);
}
}
// After
class FileRepository
{
private IFileWriter writer = null;
public FileRepository() : this( new MyXmlFileWriter() ){ }
public FileRepository(IFileWriter writer)
{
this.writer = writer;
}
public void Save( string path, string xml)
{
this.writer.WriteData(path, xml);
}
}
So what did we do?
- Have a default constructor which uses the normal type
- Have a constructor that takes an
IFileWriter
type
- Used an instance field to hold the referenced object.
Then its a case of writing a unit test for the FileRepository
and checking that the method is called:
[TestClass]
class FileRepositoryTests
{
private FileRepository repository = null;
[TestInitialize()]
public void Initialize()
{
this.repository = new FileRepository( new FakeFileWriter() );
}
[TestMethod]
public void WriteData_WhenCalled_ExpectSuccess()
{
// Arrange
var target = repository;
// Act
var actual = repository.Save(null,null);
// Assert
Assert.IsTrue(actual);
}
}
Okay, but here, are we really testing the FileRepository
or the FakeFileWriter
? We're testing the FileRepository
as our other tests are testing the FakeFileWriter
separately. This class - FileRepositoryTests
would be more useful to test the incoming parameters for nulls.
The fake is not doing anything clever - no parameter validation, no I/O. It is just sitting in so that the FileRepository can save content any work. Its purpose is two-fold; To speed up unit testing significantly and to not disrupt the state of a system.
If this FileRepository had to read the file as well, you could implement a IFileReader as well, (which is a bit extreme), or just stored the last written filePath/xml to a string in memory and retrieve that instead.
So with the basics over with - how do you approach this?
On a large project, which requires a lot of refactoring, it is always best to incorporate unit testing to any class that undergoes a DI change. In theory, your data should not be committed to hundreds of locations [within your code], but pushed through a few key locations. Locate them in the code and add an interface for them. One trick I've used is to hide each DB or index-like source behind an interface like this:
interface IReadOnlyRepository<TKey, TValue>
{
TValue Retrieve(TKey key);
}
interface IRepository<TKey, TValue> : IReadOnlyRepository<TKey, TValue>
{
void Create(TKey key, TValue value);
void Update(TKey key, TValue);
void Delete(TKey key);
}
Which sets you up to retrieve from data sources in a very generic way. You can switch from an XmlRepository
to DbRepository
by only replacing where it is injected. This can be extremely useful for a project migrating from one data source to another without affecting the internals of a system. It can be handful changing XML manipulation to using objects, but it is much easier to maintain and implement new functionality with this approach.
The only other advice I can give is do 1 data source at a time and stick do it. Resist the temptation to do too many in one go. If you really end up having to save to files, DB and web service in one hit, use the Extract Interface, fake the calls and return nothing. It is a real juggling act to do lots in one go, but you can slot them back in more easily than starts from first principles.
Good luck!