Recently, at work, I got into a passionate discussion about whether we should hide things like StringUtils behind a facade.
I was helping a coworker add some tests around a monstrous method and noted that it used StringUtils static methods quite a few times in the method. Initially I thought that this was a utility class that we had written, so rather than changing the class to have non-static methods, I opted to try to hide it behind a facade (in this case it was a simple pass-through object, but the methods on this object were non-static, thus allowing me to mock it with Mockito). Upon investigation, it turned out we were using commons.lang StringUtils. Still, I thought it might be good to put it behind a real object so we could mock it out.
I explained to my colleague that the reason I wanted to do this was so that we didn’t have to set up our tests with realistic data. We could pass in very fake data and we could make sure that we were only testing the method in question (which was VERY important since it was a behemoth). By putting it behind the facade, we were able to make it return whatever we wanted.
We spent the morning getting this all set up and she was ready to start off on her own in the afternoon. Later that afternoon, she was talking with the other developer (and our boss), who works in the code base in question regularly about the tests she was writing. He did not agree at all about us mocking out StringUtils.
A very heated discussion ensued. In the end, since I didn’t work in the code in question, I left it up to them how they wanted to test it.
After the discussion, I was reflecting on why I felt so strongly about mocking out StringUtils, and I think I have pinpointed some of the reasons.
- The method under test was doing a LOT of things, including calling out to StringUtils nearly a dozen times for different things. By mocking out StringUtils I felt more comfortable that we were tackling the code that was in the method without having to worry about what StringUtils was going to do with the data we passed it. I was trying to reduce the complexity of the method in question by ensuring I was using things I had complete control of.
- I saw StringUtils as a collaborator and wanted to make sure that I was testing only the method in question, and not the collaborating code. I believe I saw it this way due to point 1.
- I knew that we had some other classes in the code base that had a bunch of static methods on them. I would treat these the same way. Rather than treating each different class with static methods differently based on some gut feeling, I wanted to be consistent and treat any class that offered static methods the same way. This allows me to focus on the tests, rather than spending mental cycles on deciding whether this particular things should be mocked or not.
- Hiding a 3rd party tool behind a facade is a best practice I have heard many times. It allows you to swap out the tool for a new one with little effort. It can also make it much easier to upgrade to a new version, even if the new version has changed the API, since if everyone using it is actually using the facade, you only need to make the change in one place, and everyone else just continues using the facade in the same way as they always had.
Simply refactoring the monolithic method into smaller methods that could do each individual task would have left me testing it much differently.