Reputation: 366
I'm performing this TDD kata excercise: http://osherove.com/tdd-kata-1
I produced following code (points from 1 to 5 in this excercise - I have unit tests for it):
public class StringCalculator
{
private readonly string[] _defaultSeparators = { ",", "\n" };
public int Add(string numbers)
{
// Parser section (string to list of ints)
var separators = _defaultSeparators;
var isSeparatorDefinitionSpecified = numbers.StartsWith("//");
if (isSeparatorDefinitionSpecified)
{
var endOfSeparatorDefinition = numbers.IndexOf('\n');
var separator = numbers.Substring(2, endOfSeparatorDefinition - 2);
numbers = numbers.Substring(endOfSeparatorDefinition);
separators = new[] { separator };
}
var numbersArray = numbers.Split(separators, StringSplitOptions.RemoveEmptyEntries);
var numbersArrayAsInts = numbersArray.Select(int.Parse).ToArray();
// Validator section
var negativeNumbers = numbersArrayAsInts.Where(c => c < 0).ToArray();
if (negativeNumbers.Any())
{
throw new Exception(string.Format("negatives not allowed ({0})", string.Join(", ", negativeNumbers)));
}
return numbersArrayAsInts.Sum();
}
}
Now I want to refactor code to something like this:
public int Add(string numbers)
{
var numbersAsInts = CalculatorNumbersParser.Parse(numbers);
CalculatorNumbersValidator.Validate(numbersAsInts);
return numbersAsInts.Sum();
}
How I should plan refactor to properly refactor my code and unit tests?
I think that I should move part of tests to new created implementations classes tests (CalculatorNumbersParserTests and CalculatorNumbersValidatorTests), change some existing tests and add tests for Parse and Validate method execution.
But what is the correct way to do this without breaking the tests?
Upvotes: 3
Views: 166
Reputation: 10859
I think @Sam Holder has covered most things. The one thing that I would add is that when you are re-factoring your code you should mark any classes you create that you aren't going to write specific tests for as internal
(I'm assuming you're using .net), so that they aren't visible outside the assembly they are contained in.
I tend to think public
classes should be tested in their own right, since they can be instantiated easily by other code the references your assembly. Whereas, internal
classes on the other hand can be thought of as implementation details, and can usually be tested through the assemblies public interface. There are of course exceptions to this, depending on what you're doing / complexity of the code etc. but that's my general rule.
Upvotes: 3
Reputation: 32954
I would caution against moving the tests as if you do this then your tests are tied to the implementation, which means they are very brittle and so you will have to change your tests every time you want to change your implementation. This can quickly become expensive when you have a large code base and can become a prohibiting factor in making changes.
Your existing tests should specify the behaviour of your string calculator and so you can refactor your implementation to anything as long as you keep the desired behaviour.
I tend to think of a unit as a 'unit of behaviour' and it may take a few classes to implement this.
Things might change if you were to place some classes in a different assembly, at which point you would probably want to make some new tests along side the new assembly, to ensure the behaviour of those components is not changed unexpectedly, but in this case I doubt you will be doing this.
Things might also change if you start to reuse the classes in several places, at which point you may want separate tests to specify the behaviour of the classes independently of their use in the places.
Upvotes: 4