A method is an abstraction

The name of a method must have a higher level of abstraction than the body for the method to be worth it. The name should explain what the method does but not how. If the name communicates exactly the same than its code, it could be a sign that the method does not pay off and should be inlined.

Whenever we extract a method from a bunch of lines of code, we are adding a little abstraction to our code. If the extraction happens too early, then we are adding wrong abstractions to the code. Those premature abstractions get in our way to a better design, often hiding duplication and deeper design problems. Lots of small methods does not necessarily mean better design.

I extract a method only when my tests are green and it's really obvious the responsibility of those lines. When I am done implementing the branch of the solution I am test-driving and it's easy to find a name for the new method.  I don't create new methods to make a test pass. Methods appear later.

Code below is an example of premature method extraction. Right after the first test passed the developer created this method:

  2. private boolean isValidArithmeticExpression(String input) {
  3. return input != null;
  4. }

The body of the method is perfectly understandable without the method. There was no duplication of the statement and there were just a few lines in the production code. Definitely too risky, too early, too complex.

An example of a wrong encapsulation:

  2. public string splitBySpace(string expression){
  3. return expression.split(" ");
  4. }

In this example the method's name says exactly what one can read in the body.

The same applies to helper methods in the tests. This is an example of a wrong encapsulation:

  2. private void conductCalculationAndAssertResult(String equation, int expectedResult) {
  3. int result = calculate(equation);
  4. assertEquals(expectedResult, result);
  5. }

Implementing a fake repository

Lately I am using fake objects more than the other common kinds of doubles - stubs, spies and mocks. This is quite recent, I used to prefer stubs, spies or mocks. I find that fakes make my tests less fragile in the case of outside-in development when it comes to changes in the design. The problem is that there could be defects in the implementation of the fake objects. A tricky one may appear in the case of in-memory repositories:

  1. public class InMemoryRepo : JobRepository {
  2. readonly List jobs = new List();
  3. public void Add(Job job)
  4. {
  5. jobs.Add(job);
  6. }
  7. public void Update(Job job)
  8. {
  9. // TODO
  10. }
  11. public List FindBy(string ownerId)
  12. {
  13. return jobs.Where(job => job.IsOwnedBy(ownerId)).ToList();
  14. }
  15. }

This implementation is misleading. If we are test-driving a service expecting it to update a Job instance, we may forget to invoke the repository's Update method and it would still work.

  1. [TestClass]
  2. public class ServiceTest {
  3. [Test]
  4. public void AssignsOwnerToJob()
  5. {
  6. var service = Factory.CreateJobServiceWithFakeRepository();
  7. var job = service.CreateJob(irrelevantTitle);
  9. service.Assign(job, irrelevantOwnerId);
  11. service.JobsOwnedBy(irrelevantOwnerId).Should().HaveCount(1);
  12. }
  13. }
  15. public class Service {
  16. private JobRepository repo {get; set;}
  18. public Service(JobRepository repo){
  19. this.repo = repo;
  20. }
  21. public Job CreateJob(string title)
  22. {
  23. var job = new Job();
  24. repo.Add(job);
  25. /*...*/
  26. return job;
  27. }
  28. public void Assign(Job job, string ownerId){
  29. /*...*/
  30. job.Assign(ownerId);
  31. /* BUG: am forgetting the call to "repo.Update(job)" right here */
  32. }
  33. public List JobsOwnedBy(string ownerId){
  34. return repo.FindBy(ownerId);
  35. }
  36. }

The problem is that the in-memory repository is using the same object references all the time. When an object is passed in as an argument, it's actually a copy of the reference. However if the repository was accessing a database it would most likely return different instances (references). A better implementation of the repository would be this one:

  1. public class InMemoryRepo : JobRepository {
  2. readonly List jobs = new List();
  3. public void Add(Job job)
  4. {
  5. /* clone: the object added to the repo
  6.   is different to the given one */
  7. jobs.Add(job.Clone());
  8. }
  9. public void Update(Job job)
  10. {
  11. /* find the given job,
  12.   remove it from the collection and
  13.   add a clone of the given one */
  14. }
  15. public List FindBy(string ownerId)
  16. {
  17. return jobs.Where(job => job.IsOwnedBy(ownerId)).ToList();
  18. }
  19. }

Conclusion: the behavior of the fake should be the same than the original when it comes to handling object references.