As developers, we are often faced with legacy code bases that were never designed with unit testing in mind. Of course, we now know the value of unit tests and we want to test as much as possible. A good place to start implementing tests in old code is to write tests around each bug and feature. Do not try to refactor the entire pile of rubble, but instead begin with tests around just the code you are adding or changing.
Here I will show you a simple technique to refactor “just enough” to get some tests running without introducing unacceptable risk in your modifications. The examples with be in C#, but the technique applies everywhere.
Consider the following class, StudentGrader. This class is used to generate end-of-term grades for a student. It has one mega-method that does everything:
public class StudentGrader
{
public double GradeStudent(int studentID)
{
//hard dependency on this "thing" that works with student grades
GradeManager grdMngr = new GradeManager();
List allGrades = grdMngr.GetStudentGrades(studentID);
//Actual business logic
double finalGrade = 0.0;
//Throw out highest and lowest grades
var newAllGrades = allGrades.OrderBy(g => g.Grade).Skip(1).Take(allGrades.Count - 2).ToList();
finalGrade = newAllGrades.Average(g => g.Grade);
//Direct database access
string cnxnStr = ConfigurationManager.ConnectionStrings["Foo"].ConnectionString;
using(SqlConnection con = new SqlConnection(cnxnStr))
using(SqlCommand cmd = con.CreateCommand())
{
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = "usp_Student_CreateFinalGrade";
SqlParameter par_ID = new SqlParameter("@ID", SqlDbType.Int);
par_ID.Value = studentID;
cmd.Parameters.Add(par_ID);
SqlParameter par_Grade = new SqlParameter("@Grade", SqlDbType.Decimal);
par_Grade.Value = finalGrade;
cmd.Parameters.Add(par_Grade);
cmd.ExecuteNonQuery();
}
return finalGrade;
}
}
There are problems with this code that prevent us from unit testing it. True unit tests do not hit real databases or web services. The code should only be testing business logic so we need to take the dependencies out of GradeStudent so we can make sure it calculates a grade correctly, and nothing more.
Ideally we would utilize dependency injection to acquire the GradeManager instance, and a repository that does the stored procedure call, but this may introduce too much risk and effort. We need a way to minimize risk and still allow test-confident code. The simple way to do this is to break out the dependencies we don’t want into separate virtual methods that are easily mocked.
If we were to write a test around GradeStudent right now, we have no way of preventing hits on a database, and possibly a web service, after all, we can’t see what the GradeManager is doing. For all we know it could be hitting an HTTP service, a database AND an in-memory cache.
Let’s refactor to facilitate testing:
public class StudentGrader
{
public virtual double GradeStudent(int studentID)
{
List allGrades = GetStudentGrades(studentID);
//Actual business logic
double finalGrade = 0.0;
var newAllGrades = allGrades.OrderBy(g => g.Grade).Skip(1).Take(allGrades.Count - 2).ToList();
finalGrade = newAllGrades.Average(g => g.Grade);
CreateFinalGrade(studentID, finalGrade);
return finalGrade;
}
public virtual List GetStudentGrades(int studentID)
{
GradeManager grdMngr = new GradeManager();
return grdMngr.GetStudentGrades(studentID);
}
public virtual bool CreateFinalGrade(int studentID, double finalGrade)
{
string cnxnStr = ConfigurationManager.ConnectionStrings["Foo"].ConnectionString;
using(SqlConnection con = new SqlConnection(cnxnStr))
using(SqlCommand cmd = con.CreateCommand())
{
cmd.CommandType = CommandType.StoredProcedure;
cmd.CommandText = "usp_Student_CreateFinalGrade";
SqlParameter par_ID = new SqlParameter("@ID", SqlDbType.Int);
par_ID.Value = studentID;
cmd.Parameters.Add(par_ID);
SqlParameter par_Grade = new SqlParameter("@Grade", SqlDbType.Decimal);
par_Grade.Value = finalGrade;
cmd.Parameters.Add(par_Grade);
cmd.ExecuteNonQuery();
}
return true;
}
}
Notice how GradeStudent is now much simpler and much less concerned with things that are not the business logic of calculating a grade.
Now, you might be thinking, “great, but all you did was move the code around”. I did move the code around, but I also made these discrete operations virtual, which means they can be easily mocked in a unit test. In this example I am using Moq, which you can get via Nuget: Install-Package Moq -Version 4.5.28
[TestMethod]
public void CalculateFinalGradeTest()
{
//This creates a mock of StudentGrader and tells it to call the real implementation
//if a Setup is not provided
var mockStudentGrader = new Mock(){CallBase = true};
//When we call GetStudentGrades, we get a known list of grades, no worries about
//a real database connection or unexpected updates throwing off the results.
mockStudentGrader.Setup(m => m.GetStudentGrades(It.IsAny()).Returns(
new List{
new Grades { Grade = 88},
new Grades { Grade = 89},
new Grades { Grade = 98},
new Grades { Grade = 77},
new Grades { Grade = 91}
}
);
mockStudentGrader.Setup(m => m.CreateFinalGrade(It.IsAny(), It.IsAny())
.Returns(true);
//GradeStudent() will not be mocked because we did not provide a Setup
double finalgrade = mockStudentGrader.Object.GradeStudent(43);
Assert.AreEqual(89.3333333333333, finalgrade);
}
Now GradeStudent() is focused on the business logic of calculating grades and the next time the logic changes, we can be sure we have not broken it, or we will know we have broken it because the test will fail.
The key points are:
- Remove dependencies from methods, focus on one unit of business logic.
- Do not
newup anything that contains its own logic within your method. Delegate that task to a new, virtual method so that you can mock its implementation. - Make these changes on the areas of code you are touching anyway, the best way to refactor is also probably too risky unless you have the appropriate amount of time to do it.
contrived example 