Today I watched an introduction to test driven design using angular.js' testing tools.

In general it is a nice introductory talk, however one of the examples used got me thinking. The scenario (about 33 minutes into the video) is a notifier class that batches notifications and sends them to the user when a certain limit is hit.

~~~~{.javascript} Notifier.send = function (user, message) { var email = user.getDetails().email // enqueue email + message for eventual delivery by the backend ... } ~~~~ Now it is reasoned that the notifier having to know about the user's internal structure is a bad thing (making a reference to the law of Demeter). This leads to the following refactoring at about 33:45.

~~~~{.javascript} Notifier.send = function (email, message) { // enqueue email + message for eventual delivery by the backend ... } ~~~~

I'd guess the real motivation for changing this is the fact that when testing the send method you need to stub out a user object together with the details. This makes the test look worse and thus creates an incentive to find another solution.

Don't get me wrong, the signature change definitely has an advantage. The notifier doesn't need to know about the user at all - but there are some drawbacks to this:

1. The notifier will never be able to send notifications via anything but email (which would be a perfectly valid design decision, if consciously made).
2. The notifier is unable to use user-specific preferences like preferred sending times or per user batch settings.
3. Calling code needs to know about the user's email address.

I think 2. would be a pretty common addition to the Notifier. Given that the Notifier doesn't know the user we would now need to pass in the user's custom batch size together with the address. This makes the issue raised in 3. even more obvious. In my opinion code that wants to notify the user of some event in the application (like "your encoding job has completed") does have less of a reason, not more, to know the user's implementation details. Having to change every invocation from

~~~~{.javascript} send(user.getDetails().email, "your encoding job…") ~~~~

to

~~~~{.javascript} send(user.getDetails().email, user.getDetails().batchSize, "your encoding job…") ~~~~

is definitely worse than just updating the implementation of Notifier.send.

I think this is an important point to think about, because in general when making code more testable you always walk away with the feeling of having improved the code. And on some level you might - you just have to keep in mind that testability is not the only metric of quality.