The Simple Story Paradox

I’ve recently been following the #isTDDDead debate between Kent Beck (@kentbeck), David Heinemeier Hansson (@dhh), and Martin Fowler (@martinfowler) with some interest. I think that it’s particularly beneficial that ideas, which are often taken for granted, can be challenged in a constructive manner. That way you can figure out if they stand up to scrutiny or fall down flat on their faces.

The discussion began with @dhh making the following points on TDD and test technique, which I hope I’ve got right. Firstly, the strict definition of TDD includes the following:
 
 

  1. TTD is used to drive unit tests
  2. You can’t have collaborators
  3. You can’t touch the database
  4. You can’t touch the File system
  5. Fast Unit Tests, complete in the blink of an eye.

He went on to say that you therefore drive your system’s architecture from the use of mocks and in that way the architecture suffers damage from the drive to isolate and mock everything, whilst the mandatory enforcement of the ‘red, green, refactor’ cycle is too prescriptive. He also stated that a lot of people mistake that you can’t have confidence in your code and you can’t deliver incremental functionality with tests unless you go through this mandated, well paved road of TDD.

@Kent_Beck said that TDD didn’t necessarily include heavy mocking and the discussion continued…

I’ve paraphrased a little here; however, it was the difference in the interpretation and experience of using TDD that got me thinking.

Was it really a problem with TDD or was it with @dhh’s experience of other developer’s interpretation of TDD? I don’t want to put words into @dhh’s mouth, but it seems like the problem is the dogmatic application of the TDD technique even when it isn’t applicable. I came away with the impression that, in certain development houses, TDD had degenerated into little more than Cargo Cult Programming.

RichardFeynmanThe term Cargo Cult Programming seems to derive from a paper written by someone whom I found truly inspirational, the late Professor Richard Feynman. He presented a paper entitled Cargo Cult Science – Some remarks on science, pseudoscience and learning how not to fool yourself as part of Caltech’s 1974 commencement address. This later became part of his autobiography: Surely you must be joking Mr Feynman, a book that I implore you to read.

In it, Feynman highlights experiments from several pseudosciences, such as educational science, psychology, parapsychology and physics, where the scientific approach of keeping an open mind, questioning everything and looking for flaws in your theory have been replaced by belief, ritualism and faith: a willingness to take other peoples results for granted in lieu of an experimental control.

Taken from the 1974 paper, Feynman sums up Cargo Cult Science as:

“In the South Seas there is a cargo cult of people. During the war they saw airplanes land with lots of good materials, and they want the same thing to happen now. So they’ve arranged to imitate things like runways, to put fires along the sides of the runways, to make a wooden hut for a man to sit in, with two wooden pieces on his head like headphones and bars of bamboo sticking out like antennas–he’s the controller–and they wait for the airplanes to land. They’re doing everything right. The form is perfect. It looks exactly the way it looked before. But it doesn’t work. No airplanes land. So I call these things cargo cult science, because they follow all the apparent precepts and forms of scientific investigation, but they’re missing something essential, because the planes don’t land.”

You can apply this idea to programming where you’ll find teams and individuals carrying out ritualised procedures and using techniques without really understanding the theory behind them in the hope that they’ll work and because they are the ‘right thing to do’.

In the second talk in the series @dhh came up with an example of what he called “test induced design damage” and at this I got excited because it’s something I’ve seen a number of times. The only reservation I had about the gist code was that to me it didn’t seem to result from TDD, that argument seems a little limited; I’d say that it was more a result of Cargo Cult Programming and that’s because in the instances where I’ve come across this example TDD wasn’t used.

If you’ve seen the Gist, you may know what I’m talking about; however, that code is in Ruby, which is something I’ve little experience of. In order to explore this in more detail, I thought that I’d create a Spring MVC version and go from there.

The scenario here is one where we have a very simple story: all the code does is to read an object from the database and place it into the model for display. There’s no additional processing, no business logic and no calculations to perform. The agile story would go something like this:

Title: View User Details
As an admin user
I want to click on a link
So that I can verify a user's details

In this ‘Proper’ N tier sample, I have a User model object, a controller and service layer and DAO together with their interfaces and tests.

And, there’s the paradox: you set out to write the best code you possibly can to implement the story, using the well known and probably most popular MVC ‘N’ layer pattern and end up with something that’s total overkill for such a simple scenario. Something, as @jhh would say, is damaged.

In my sample code, I’m using the JdbcTemplate class to retrieve a user’s details from a MySQL database, but any DB access API will do.

This is the sample code demonstrating the conventional, ‘right’ way of implementing the story; prepare to do a lot of scrolling…

public class User { 
 
  public static User NULL_USER = new User(-1, "Not Available", "", new Date()); 
 
  private final long id; 
 
  private final String name; 
 
  private final String email; 
 
  private final Date createDate; 
 
  public User(long id, String name, String email, Date createDate) { 
    this.id = id; 
    this.name = name; 
    this.email = email; 
    this.createDate = createDate; 
  } 
 
  public long getId() { 
    return id; 
  } 
 
  public String getName() { 
    return name; 
  } 
 
  public String getEmail() { 
    return email; 
  } 
 
  public Date getCreateDate() { 
    return createDate; 
  } 
}

 

@Controller 
public class UserController { 
 
  @Autowired 
  private UserService userService; 
 
  @RequestMapping("/find1") 
  public String findUser(@RequestParam("user") String name, Model model) { 
 
    User user = userService.findUser(name); 
    model.addAttribute("user", user); 
    return "user"; 
  } 
}

 

public interface UserService { 
 
  public abstract User findUser(String name); 
 
}

 

@Service 
public class UserServiceImpl implements UserService { 
 
  @Autowired 
  private UserDao userDao; 
 
  /** 
   * @see com.captaindebug.cargocult.ntier.UserService#findUser(java.lang.String) 
   */ 
  @Override 
  public User findUser(String name) { 
    return userDao.findUser(name); 
  } 
}

 

public interface UserDao { 
 
  public abstract User findUser(String name); 
 
}

 

@Repository 
public class UserDaoImpl implements UserDao { 
 
  private static final String FIND_USER_BY_NAME = "SELECT id, name,email,createdDate FROM Users WHERE name=?"; 
 
  @Autowired 
  private JdbcTemplate jdbcTemplate; 
 
  /** 
   * @see com.captaindebug.cargocult.ntier.UserDao#findUser(java.lang.String) 
   */ 
  @Override 
  public User findUser(String name) { 
 
    User user; 
    try { 
      FindUserMapper rowMapper = new FindUserMapper(); 
      user = jdbcTemplate.queryForObject(FIND_USER_BY_NAME, rowMapper, name); 
    } catch (EmptyResultDataAccessException e) { 
      user = User.NULL_USER; 
    } 
    return user; 
  } 
}

If you take a look at this code, paradoxically it looks fine; in fact it looks like a classic text book example of how to write an ‘N’ tier MVC application. The controller passes responsibility for sorting out the business rules to the service layer and the service layer retrieves data from the DB using a data access object, which in turn uses a RowMapper<> helper class to retrieve a User object. When the controller has a User object it injects it into the model ready for display. This pattern is clear and extensible; we’re isolating the database from the service and the service from the controller by using interfaces and we’re testing everything using both JUnit with Mockito, and integration tests. This should be the last word in text book MVC coding, or is it? Let’s look at the code.

Firstly, there’s the unnecessary use of interfaces. Some would argue that it’s easy to switch database implementations, but who ever does that? 1 plus, modern mocking tools can create their proxies using Class definitions so, unless your design specifically requires multiple implementations of the same interface, then using interfaces is pointless.

Next, there is the UserServiceImpl, which is a classic example of the lazy class anti-pattern, because it does nothing except pointlessly delegate to the data access object. Likewise,the controller is also pretty lazy as it delegates to the lazy UserServiceImpl before adding the resulting User class to the model: in fact, all these classes are examples of the lazy class anti pattern.

Having written some lazy classes, they are now needlessly tested to death, even the non-event UserServiceImpl class. It’s only worth writing tests for classes that actually perform some logic.

public class UserControllerTest { 
 
  private static final String NAME = "Woody Allen"; 
 
  private UserController instance; 
 
  @Mock 
  private Model model; 
 
  @Mock 
  private UserService userService; 
 
  @Before 
  public void setUp() throws Exception { 
 
    MockitoAnnotations.initMocks(this); 
 
    instance = new UserController(); 
    ReflectionTestUtils.setField(instance, "userService", userService); 
  } 
 
  @Test 
  public void testFindUser_valid_user() { 
 
    User expected = new User(0L, NAME, "aaa@bbb.com", new Date()); 
    when(userService.findUser(NAME)).thenReturn(expected); 
 
    String result = instance.findUser(NAME, model); 
    assertEquals("user", result); 
 
    verify(model).addAttribute("user", expected); 
  } 
 
  @Test 
  public void testFindUser_null_user() { 
 
    when(userService.findUser(null)).thenReturn(User.NULL_USER); 
 
    String result = instance.findUser(null, model); 
    assertEquals("user", result); 
 
    verify(model).addAttribute("user", User.NULL_USER); 
  } 
 
  @Test 
  public void testFindUser_empty_user() { 
 
    when(userService.findUser("")).thenReturn(User.NULL_USER); 
 
    String result = instance.findUser("", model); 
    assertEquals("user", result); 
 
    verify(model).addAttribute("user", User.NULL_USER); 
  } 
 
}

 

public class UserServiceTest { 
 
  private static final String NAME = "Annie Hall"; 
 
  private UserService instance; 
 
  @Mock 
  private UserDao userDao; 
 
  @Before 
  public void setUp() throws Exception { 
 
    MockitoAnnotations.initMocks(this); 
 
    instance = new UserServiceImpl(); 
 
    ReflectionTestUtils.setField(instance, "userDao", userDao); 
  } 
 
  @Test 
  public void testFindUser_valid_user() { 
 
    User expected = new User(0L, NAME, "aaa@bbb.com", new Date()); 
    when(userDao.findUser(NAME)).thenReturn(expected); 
 
    User result = instance.findUser(NAME); 
    assertEquals(expected, result); 
  } 
 
  @Test 
  public void testFindUser_null_user() { 
 
    when(userDao.findUser(null)).thenReturn(User.NULL_USER); 
 
    User result = instance.findUser(null); 
    assertEquals(User.NULL_USER, result); 
  } 
 
  @Test 
  public void testFindUser_empty_user() { 
 
    when(userDao.findUser("")).thenReturn(User.NULL_USER); 
 
    User result = instance.findUser(""); 
    assertEquals(User.NULL_USER, result); 
  } 
}

 

public class UserDaoTest { 
 
  private static final String NAME = "Woody Allen"; 
 
  private UserDao instance; 
 
  @Mock 
  private JdbcTemplate jdbcTemplate; 
 
  @Before 
  public void setUp() throws Exception { 
 
    MockitoAnnotations.initMocks(this); 
 
    instance = new UserDaoImpl(); 
    ReflectionTestUtils.setField(instance, "jdbcTemplate", jdbcTemplate); 
  } 
 
  @SuppressWarnings({ "unchecked", "rawtypes" }) 
  @Test 
  public void testFindUser_valid_user() { 
 
    User expected = new User(0L, NAME, "aaa@bbb.com", new Date()); 
    when(jdbcTemplate.queryForObject(anyString(), (RowMapper) anyObject(), eq(NAME))).thenReturn(expected); 
 
    User result = instance.findUser(NAME); 
    assertEquals(expected, result); 
  } 
 
  @SuppressWarnings({ "unchecked", "rawtypes" }) 
  @Test 
  public void testFindUser_null_user() { 
 
    when(jdbcTemplate.queryForObject(anyString(), (RowMapper) anyObject(), isNull())).thenReturn(User.NULL_USER); 
 
    User result = instance.findUser(null); 
    assertEquals(User.NULL_USER, result); 
  } 
 
  @SuppressWarnings({ "unchecked", "rawtypes" }) 
  @Test 
  public void testFindUser_empty_user() { 
 
    when(jdbcTemplate.queryForObject(anyString(), (RowMapper) anyObject(), eq(""))).thenReturn(User.NULL_USER); 
 
    User result = instance.findUser(""); 
    assertEquals(User.NULL_USER, result); 
  } 
 
}

 

@RunWith(SpringJUnit4ClassRunner.class) 
@WebAppConfiguration 
@ContextConfiguration({ "file:src/main/webapp/WEB-INF/spring/appServlet/servlet-context.xml", 
    "file:src/test/resources/test-datasource.xml" }) 
public class UserControllerIntTest { 
 
  @Autowired 
  private WebApplicationContext wac; 
 
  private MockMvc mockMvc; 
 
  /** 
   * @throws java.lang.Exception 
   */ 
  @Before 
  public void setUp() throws Exception { 
 
    mockMvc = MockMvcBuilders.webAppContextSetup(wac).build(); 
  } 
 
  @Test 
  public void testFindUser_happy_flow() throws Exception { 
 
    ResultActions resultActions = mockMvc.perform(get("/find1").accept(MediaType.ALL).param("user", "Tom")); 
    resultActions.andExpect(status().isOk()); 
    resultActions.andExpect(view().name("user")); 
    resultActions.andExpect(model().attributeExists("user")); 
    resultActions.andDo(print()); 
 
    MvcResult result = resultActions.andReturn(); 
    ModelAndView modelAndView = result.getModelAndView(); 
    Map<String, Object> model = modelAndView.getModel(); 
 
    User user = (User) model.get("user"); 
    assertEquals("Tom", user.getName()); 
    assertEquals("tom@gmail.com", user.getEmail()); 
  } 
 
}

In writing this sample code, I’ve added everything I could think of into the mix. You may think that this example is ‘over the top’ in its construction especially with the inclusion on redundant interface, but I have seen code like this.

The benefits of this pattern are that it follows a distinct design understood by most developers; it’s clean and extensible. The down side is that there are lots of classes. More classes take more time to write and,you ever have to maintain or enhance this code, they’re more difficult to get to grips with.

So, what’s the solution? That’s difficult to answer. In the #IsTTDDead debate @dhh gives the solution as placing all the code in one class, mixing the data access with the population of the model. If you implement this solution for our user story you still get a User class, but the number of classes you need shrinks dramatically.

@Controller 
public class UserAccessor { 
 
  private static final String FIND_USER_BY_NAME = "SELECT id, name,email,createdDate FROM Users WHERE name=?"; 
 
  @Autowired 
  private JdbcTemplate jdbcTemplate; 
 
  @RequestMapping("/find2") 
  public String findUser2(@RequestParam("user") String name, Model model) { 
 
    User user; 
    try { 
      FindUserMapper rowMapper = new FindUserMapper(); 
      user = jdbcTemplate.queryForObject(FIND_USER_BY_NAME, rowMapper, name); 
    } catch (EmptyResultDataAccessException e) { 
      user = User.NULL_USER; 
    } 
    model.addAttribute("user", user); 
    return "user"; 
  } 
 
  private class FindUserMapper implements RowMapper<User>, Serializable { 
 
    private static final long serialVersionUID = 1L; 
 
    @Override 
    public User mapRow(ResultSet rs, int rowNum) throws SQLException { 
 
      User user = new User(rs.getLong("id"), // 
          rs.getString("name"), // 
          rs.getString("email"), // 
          rs.getDate("createdDate")); 
      return user; 
    } 
  } 
}

 

@RunWith(SpringJUnit4ClassRunner.class) 
@WebAppConfiguration 
@ContextConfiguration({ "file:src/main/webapp/WEB-INF/spring/appServlet/servlet-context.xml", 
    "file:src/test/resources/test-datasource.xml" }) 
public class UserAccessorIntTest { 
 
  @Autowired 
  private WebApplicationContext wac; 
 
  private MockMvc mockMvc; 
 
  /** 
   * @throws java.lang.Exception 
   */ 
  @Before 
  public void setUp() throws Exception { 
 
    mockMvc = MockMvcBuilders.webAppContextSetup(wac).build(); 
  } 
 
  @Test 
  public void testFindUser_happy_flow() throws Exception { 
 
    ResultActions resultActions = mockMvc.perform(get("/find2").accept(MediaType.ALL).param("user", "Tom")); 
    resultActions.andExpect(status().isOk()); 
    resultActions.andExpect(view().name("user")); 
    resultActions.andExpect(model().attributeExists("user")); 
    resultActions.andDo(print()); 
 
    MvcResult result = resultActions.andReturn(); 
    ModelAndView modelAndView = result.getModelAndView(); 
    Map<String, Object> model = modelAndView.getModel(); 
 
    User user = (User) model.get("user"); 
    assertEquals("Tom", user.getName()); 
    assertEquals("tom@gmail.com", user.getEmail()); 
  } 
 
  @Test 
  public void testFindUser_empty_user() throws Exception { 
 
    ResultActions resultActions = mockMvc.perform(get("/find2").accept(MediaType.ALL).param("user", "")); 
    resultActions.andExpect(status().isOk()); 
    resultActions.andExpect(view().name("user")); 
    resultActions.andExpect(model().attributeExists("user")); 
    resultActions.andExpect(model().attribute("user", User.NULL_USER)); 
    resultActions.andDo(print()); 
  } 
}

The solution above cuts the number of first level classes to two: an implementation class and a test class. All test scenarios are catered for in a very few end to end integration tests. These tests will access the database, but is that so bad in this case? If each trip to the DB takes around 20ms or less then they’ll still complete within a fraction of a second; that should be fast enough.

In terms of enhancing or maintaining this code, one small single class is easier to learn than several even smaller classes. If you did have to add in a whole bunch of business rules or other complexity then changing this code into the ‘N’ layer pattern will not be difficult; however the problem is that if/when a change is necessary it may be given to an inexperienced developer who’ll not be confident enough to carry out the necessary refactoring. The upshot is, and you must have seen this lots of times, that the new change could be shoehorned on top of this one class solution leading to a mess of spaghetti code.

In implementing a solution like this, you may not be very popular, because the code is unconventional. That’s one of the reasons that I think that this single class solution is something that a lot of people would see as contentious. It’s this idea of a standard ‘right way’ and ‘wrong way’ of writing code, rigorously applied in every case, that has lead to this perfectly good design becoming a problem.

I guess that it’s all a matter of horses for courses; choosing the right design for the right situation. If I was implementing a complex story, then I wouldn’t hesitate to split up the various responsibilities, but in the simple case it’s just not worth it. I’ll therefore end by asking if any one has a better solution for the Simple Story Paradox shown above, please let me know.

 
1 I’ve worked on a project once in umpteen years of programming where the underlying database was changed to meet a customer requirement. That was many years and many thousands of miles away and the code was written in C++ and Visual Basic.

Reference: The Simple Story Paradox from our JCG partner Roger Hughes at the Captain Debug’s Blog blog.

Do you want to know how to develop your skillset to become a Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you two of our best selling eBooks for FREE!

JPA Mini Book

Learn how to leverage the power of JPA in order to create robust and flexible Java applications. With this Mini Book, you will get introduced to JPA and smoothly transition to more advanced concepts.

JVM Troubleshooting Guide

The Java virtual machine is really the foundation of any Java EE platform. Learn how to master it with this advanced guide!

Given email address is already subscribed, thank you!
Oops. Something went wrong. Please try again later.
Please provide a valid email address.
Thank you, your sign-up request was successful! Please check your e-mail inbox.
Please complete the CAPTCHA.
Please fill in the required fields.

Leave a Reply


+ one = 9



Java Code Geeks and all content copyright © 2010-2014, Exelixis Media Ltd | Terms of Use | Privacy Policy | Contact
All trademarks and registered trademarks appearing on Java Code Geeks are the property of their respective owners.
Java is a trademark or registered trademark of Oracle Corporation in the United States and other countries.
Java Code Geeks is not connected to Oracle Corporation and is not sponsored by Oracle Corporation.
Do you want to know how to develop your skillset and become a ...
Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you two of our best selling eBooks for FREE!

Get ready to Rock!
You can download the complementary eBooks using the links below:
Close