Monday, 9 December 2013

An effective logging model

Logging is a really trivial thing. At least it looks so. However, the trivial thing is just printing out a log line. What to print out and which extra information should that log contain is not that straightforward. We all have seen and used logging model where log statements have been classified into several categories. For example, the Java logging utility defines 7 different categories. Another common classification seems to be:
  1. Fatal: something extremely bad and unrecoverable happened causing the system to stop or exit;
  2. Error: something bad and unrecoverable happened but not so bad to halt the system;
  3. Warning: something bad happened but it has been recovered;
  4. Info: some important event has been received or stage reached;
  5. Debug: used to print out any sort of internal data to help debugging.
Along with this classification model there also exist logging frameworks capable of fancy things like:
  • Turning on and off each log level dynamically at run-time;
  • Redirecting log statements to separate media (e.g. files, sockets, system's log, etc.) based on the log level or the class or file the log has been generated from.
All this sounds useful, but too often the outcome is an extremely noisy and untrustworthy log file containing too many stuff and at times too many errors, even if the system was just behaving itself. Furthermore, the application is unusable because 90% of the time it's doing a printer's job instead of its. This is why all that fancy features exist; to try to filter all this noise and restore some performance back.

Logging is a really trivial thing and in my projects I keep it that way by using a really trivial logging model:
  1. Error: an error internal to the program occurred but it shouldn't have; this means that it's unrecoverable. For example we placed a try-catch block at some very high level and it caught some sort of exception. On the opposite, if we were able to write code to recover from an error then it wouldn't be an error, because it's something we can handle and dealing with it is part of the program's behavior. 
  2. Warning: an error external to the program occurred. Being outside of our control is something we knew it could occur and so we wrote defensive code. For example, the server received a malformed message from a client.
  3. Info: any other event which is meaningful in the application's domain. For example an embedded device discovered that a new software version is available. On the opposite, printing out a temporary variable's value is probably not meaningful in the application's domain.
There's no need for lots of logging level. For example, there's no need for debug logs which print out variables and other internal data just because "it could help". We should rather write unit tests. If they pass, then we know exactly how the program behave.

There's no need for fancy stuff like turning logging on and off at run time, because logging is less than 10% of the job, not the other way round.

There's no reason to remove logging when releasing the product, for the same reason. The only context where it might still be reasonable to strip logging off is embedded devices with no communications at all, but this sounds an extremely rare case; there's always a way to inspect/extract logs from a device.

In conclusion, logging is a trivial thing. We should keep it that way without adding fancy features of any sort. However, what is worth to be logged and which level it belongs to is more an art than a science.

Monday, 2 December 2013

The evil static methods

Most programming languages provide a way to define static methods. We all know what they are: functions of a class that can be globally invoked, without requiring an instance of that class. They do really sound handy, but the reality is different.
Let's have a look at the following code:

public class Person {
  static public boolean isSocialNumberValid(String socialNumber){
    // TODO: Check for valid prefixes and suffixes
    return  socialNumber.length() == 9                  &&
            Character.isLetter(socialNumber.charAt(0))  &&
            Character.isLetter(socialNumber.charAt(1))  &&
            Character.isDigit(socialNumber.charAt(2))   &&
            Character.isDigit(socialNumber.charAt(3))   &&
            Character.isDigit(socialNumber.charAt(4))   &&
            Character.isDigit(socialNumber.charAt(5))   &&
            Character.isDigit(socialNumber.charAt(6))   &&
            Character.isDigit(socialNumber.charAt(7))   &&
            Character.isLetter(socialNumber.charAt(8));
  }
}

The class Person provides a static method to check if a given string is a valid social number (the UK National Insurance Number). This method looks so useful that we're tempted to use it literally everywhere while developing an hypothetical management system for the public entities. It will soon become a weakness in that system.

Static methods kill OO's polymorphism

If we sold the system also to the US public entities we would need to extend that method but we can't because it's static. The only workaround to avoid changing every single call to that function, is to support both UK NIN and US Social Security Number, resulting in the following bad-looking code:

public class Person {
  static public boolean isSocialNumberValid(String socialNumber){
    return isUkNin(socialNumber) || isUsSsn(socialNumber);
  }
  static private boolean isUkNin(String socialNumber) {
    // TODO: Check for valid prefixes and suffixes
    return  socialNumber.length() == 9                  &&
            Character.isLetter(socialNumber.charAt(0))  &&
            Character.isLetter(socialNumber.charAt(1))  &&
            Character.isDigit(socialNumber.charAt(2))   &&
            Character.isDigit(socialNumber.charAt(3))   &&
            Character.isDigit(socialNumber.charAt(4))   &&
            Character.isDigit(socialNumber.charAt(5))   &&
            Character.isDigit(socialNumber.charAt(6))   &&
            Character.isDigit(socialNumber.charAt(7))   &&
            Character.isLetter(socialNumber.charAt(8));
  }
  static private boolean isUsSsn(String socialNumber) {
    return  socialNumber.length() == 9                &&
            Character.isDigit(socialNumber.charAt(0)) &&
            Character.isDigit(socialNumber.charAt(1)) &&
            Character.isDigit(socialNumber.charAt(2)) &&
            Character.isDigit(socialNumber.charAt(3)) &&
            Character.isDigit(socialNumber.charAt(4)) &&
            Character.isDigit(socialNumber.charAt(5)) &&
            Character.isDigit(socialNumber.charAt(6)) &&
            Character.isDigit(socialNumber.charAt(7)) &&
            Character.isDigit(socialNumber.charAt(8));
  }
}

Static method shouldn't exist at all

Static methods can be invoked without an instance of the class they are declared into. This means that they don't deal with objects of that class, hence they shouldn't be part of the class at all. An exception might be factory methods because they create instances of that class. However not even they should be static, because we won't never be able to leverage polymorphism to change the way those instances are created.

They hide dependencies

Take a look at the following method:

public class Database {
  public Person createPerson(String name, String address) {
    Person p = new Person();
    p.setName(name);
    p.setAddress(address);
    save(p);
    return p;
  }
}

It looks very clear. We call Database.createPerson() passing the name and the address of a person and it will create a new Person object, save it into the database and return the new instance. It's really that trivial, is it? It isn't! Someone didn't know that static methods are evil and wrote the following code:

public class Person {
  public void setAddress(String address) {
    this.address = address;
    this.latlon = GoogleGeoCoder.convertAddress(address);
  }
}

The usage of the static method GoogleGeoCoder.convertAddress() is hiding the important fact that Database depends on GoogleGeoCoder!

The Singleton anti-pattern

The only way to obtain the instance of a Singleton, is to invoke the usual getInstance() static method. Hence, Singletons have the very same issues that static methods have:
  1. They can't be extended: If GoogleGeoCoder was a Singleton and we called GoogleGeoCoder.getInstance(), then we would always obtain an instance of a GoogleGeoCoder; never one of its subclasses.
  2. Singletons can be hidden everywhere, even behind the most innocent API like Person.setAddress().

They make testing difficult

This is another big problem caused by static methods and singletons. We can't easily replace the real implementation with a fake one, as discussed in previous posts. This makes our life more complicated while testing.

The new operator is a static method

It might not be that evident but the lesser we use the new operator, the better it is. Let's look at the following snippet of code:

public class UserRegistrationController {
  public void register(String email, String password) {
    User user = new User();
    user.setEmail(email);
    user.setPassword(password);
    _db.createUser(user);
    sendWelcomeMessage(user)
  }
  private void sendWelcomeMessage(User u) {
    Email email = new Email();
    email.to(u.getEmail());    
    email.setSubject("Welcome");
    email.setBody("Welcome!");
    _emailSender.send(email);
  }
}

Because of the new operator, UserRegistrationController is tightly coupled with Email. If we wanted to send an SMS or invoke a social network API to send a private message we would need to write two new functions and stick some if to choose which type of welcome message to send.

Not all the new operators are equivalent

In fact, new Email() is a new operator we really want to remove; because of it UserRegistrationController depends on both the Email and some sort of EmailSender. There is no real need for this. It would be better if it depended only on one more abstract Notifier. On the other hand, new User() isn't harmful because User belongs to the application's domain.

Use factory objects but remember the Law of Demeter

Many of the new operators can be replaced by factory objects which would provide more extensibility and maintainability. However, keep in mind the Law of Demeter. In the following code we use an EmailFactory to create the welcome email, which the EmailSender will send:

public class UserRegistrationController {
  public void register(String email, String password) {
    User user = new User();
    user.setEmail(email);
    user.setPassword(password);
    _db.createUser(user);
    Email email = _emailFactory.welcomeEmail(user);
    _emailSender.send(email);
  }
}
There is no real need for UserRegistrationController to know about the EmailFactory; it only wants to send an email it shouldn't even create. EmailSender should rather provide the method sendWelcomeMessage() taking User as an argument: it would take care of creating and sending the necessary Email (either by means of a factory or manually by doing new Email() or even in some other fancy way).

Sunday, 10 November 2013

Test Driven Education [part 4 of 4]

A full Test Driven Project to learn from.

Our simple Home Security system is almost complete. We've written most of the units it is made of but to be fully complete and deployable, we still need to connect the dots. In all the previous posts, we've written unit tests as we were writing units, i.e. the core parts of the application. Since we're now going to finish it, we need to write code to cross the boundaries between the application and the environment it interacts with. We'll need to implement a concrete Database, a concrete Logger, a concrete Siren and the UI. This will give us the opportunity to understand the so called Testing Pyramid. It is a concept by which application's tests can be classified into three different categories, each one having an increasing number of tests (hence the pyramid shape):
  1. Unit tests. The base of the pyramid, where the greatest number of tests lie. They exercise the core part of the application: its units. A unit is definable as any piece of code we write (i.e. not 3rd party libraries we use). A unit tests is then a test which exercise only that single unit; every other unit it eventually interacts with will be replaced by a test double.
  2. Integration tests. Fewer than unit tests, they exercise the interaction between two or more concrete units or the interaction between a concrete unit and the operating system or, similarly, the application's execution environment.
  3. Component tests. The top of the pyramid, being the category with the smallest number of tests. They exercise the application as a whole. Every unit the application is made with will be real and the tests can only drive the application's inputs and read its outputs.
Being a simple project, Domus will have the simplest database ever; it will read/write the PIN code from/to a file and we'll call it FileDatabaseOur first requirement is: when FileDatabase is created it has to create the file and write the factory PIN code into it.

public class FileDatabaseTest extends TestCase {
  
  protected void tearDown() { 
    File file = new File("pin.dat");
    assertTrue("File couldn't be deleted", file.delete());
  }

  public void testCreatePinFileIfNotExists() throws Exception {
    // setup
    FileDatabase db = new FileDatabase("pin.dat");
    // verify
    Scanner dbScanner = new Scanner(new File("pin.dat"));
    assertEquals("0000", dbScanner.next());
  }
}

This is an Integration Test and the reason is that FileDatabase interacts directly with the execution environment (i.e. the JVM and a real file). Because of this interaction, integration tests are slower than unit tests. It might not sound like a big issue but as soon as the integration tests get more and more, running all the tests will take longer and longer, until they will eventually take so much time that we'll be annoyed by running them or, anyway, we'll spend lot of time waiting rather than implementing features. This should not make us think that integration tests are bad. We need them. We just want run them less frequently than unit tests, to not slow us down.
The following is the implementation of FileDatabase constructor:

  public FileDatabase(String pinFileName) throws IOException {
    FileWriter pinFile = new FileWriter(pinFileName);
    pinFile.write("0000");
    pinFile.close();
  }

To keep this short, I'll skip over the full implementation of FileDatabase (you can find all the code on git, also for FileLogger and FileSiren). To launch Domus, we'll also need a Main file which will wire all the parts together. It will also implement a classic command line interface printing out a menu with all the choices, which for our simple project will be:
  • 0, to quit the program
  • 1, to add a sensor
  • 2, to engage the alarm
  • 3, to disengage the alarm
  • 4, to trigger a sensor
  • 5, change pin code
The implementation of Main can be found on git. What's more important to look at is one component test for Domus. This test exercises the whole Domus, by adding a sensor, engaging the alarm and then triggering the sensor; the expected outcome is the activation of the siren, which is just a matter of writing 1 into the file siren.out (as often happens for /dev files on Unix platforms).

public class DomusTest extends TestCase {
  
  protected void tearDown() { 
    assertTrue(new File("pin.dat").delete());
    assertTrue(new File("log.txt").delete());
    assertTrue(new File("siren.out").delete());
  }

  public void testActivateSiren() throws Exception {
    // setup
    StringBuffer commands = new StringBuffer();
    commands.append("1\n"); /* Add a sensor */
    commands.append("2\n"); /* Engage the alarm */
    commands.append("0000\n");
    commands.append("4\n"); /* Trigger a sensor */
    commands.append("0\n");
    commands.append("0\n"); /* Quit the program */
    System.setIn(new ByteArrayInputStream(commands.toString().getBytes()));
    // exercise
    Main.main(new String[] {"pin.dat", "log.txt", "siren.out"});
    // verify
    Scanner sirenScanner = new Scanner(new File("siren.out"));
    assertEquals(1, sirenScanner.nextInt());
  }

}


Two other component tests can be written to verify that the user can:

  1. change the PIN code
  2. disengage the alarm.
You can find these tests on git. What is important to highlight is that these three tests are the only component tests needed. Looking at how many tests we've written so far, we can finally understand what the Testing Pyramid is:
  • 3 component tests
  • 8 integration tests
  • 16 unit tests
As expected, in a well tested codebase, the majority of the tests are unit tests, because they exercise all the workers of a system, proving that each one is doing its job; a lower number of integration tests prove that the peripheral parts of the system interacts properly with the execution environment; finally, a small set of component tests prove that everything has been wired correctly.

Sunday, 13 October 2013

Test Driven Education [part 3 of 4]

A full Test Driven Project to learn from.

In the last post, we've seen what the Dependency Injection is and, particularly, how it makes unit testing easier: by injecting fake collaborators, we fully control inputs and outputs of the class being tested. In this third part we're going to discover five different ways of faking collaborators, commonly known as Test Doubles:
  • Dummy Object
  • Stub Object
  • Fake Object
  • Spy Object
  • Mock Object
As usual, we'll extend Domus to fulfill new requirements. In particular, we want to persist the PIN code into a database to survive system reboots and we want to produce logs reporting relevant events such alarm engagement and disengagement. To satisfy this two requirements, we'll introduce a Database and a Logger.


Stub Object

The first Test Double to talk about is the Stub Object: its only responsibility is to return known values (fixed or configurable) to its callers; therefore stubs suite very well when an object serves as an input to the class being tested.
We need a stub object to redefine the requirement of all the tests dealing with the PIN code. The new requirement, in fact, is that SecurityAlarm shall validate the PIN entered by the user against the one persisted into the database (for size reasons here there is only the code of testEngageAlarm()):

  public void testEngageAlarm() {
    // setup
    StubDatabase db = new StubDatabase();
    SecurityAlarm alarm = new SecurityAlarm(db);
    // exercise
    alarm.engage("0000");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }

All these tests now don't compile anymore, as SecurityAlarm now expects a Database to be injected, so we define the new constructor:

  public SecurityAlarm(Database db) {
  }

All the previous tests now compiles, however all the tests we didn't change (not dealing with the PIN code) aren't compiling anymore, because there is no empty constructor for SecurityAlarm. This gives us the opportunity to talk about dummies.

Dummy Object

Dummies are objects whose only responsibility is to shut the compiler up. They concretely implement their interfaces to pass the type checking at compilation time, but they shouldn't be invoked at all by the class being tested. Bear in mind that the null keyword is considered a dummy object and it is more than welcomed in unit tests as it highlights that a particular collaborator is not involved in that particular unit test (hence, the cause of failure should be elsewhere). However, the class being tested might not allow null to be passed; in these cases, a Null Object needs to be used. As said, it concretely implements its interface but either does nothing (i.e. empty methods) or makes the test failing, depending on the context and the taste.
To make our unit tests compiling again, we'll use the null keyword wherever required, for example in testAlarmNotEngagedAtStartup():

  public void testAlarmNotEngagedAtStartup() {
    // setup
    SecurityAlarm alarm = new SecurityAlarm(null);
    // verify
    assertFalse("Alarm engaged", alarm.isEngaged());
  }

With these changes the tests are now compiling again. They still pass, but this shouldn't confuse us; SecurityAlarm is still satisfying its requirements:
  • Engaging/Disengaging the alarm
  • Activating the sirens
  • etc
The fact is: we're now doing a refactoring. When we refactor some code we move from a software that satisfies its requirements to a different version which still satisfies its requirements. So, let's keep refactoring SecurityAlarm and remove all references to the private variable _pin and the DEFAULT_PIN constant:

public class SecurityAlarm implements Sensor.TriggerListener {
  ...
  private Database _db;
  ...
  public SecurityAlarm(Database db) {
    _db = db;
  }

  private boolean isPinValid(String pin) {
    return pin.equals(_db.getPin());
  }

  ...

  public void changePinCode(String oldPin, String newPin) {
    if (isPinValid(oldPin)) {

    }
  }
  ...
}

Most of the test are still passing but others, like testChangePinCode(), are now failing which means that something is going wrong. This brings us to a new requirement and a new Test Double.

Fake Object

Fakes are the next step toward the real implementation; they pretend very well to be what they say to be. They're more clever than stubs because they start having some degree of logic driving their return values, possibly function of other properties that collaborators might even set. A FakeDatabase is then what we need to make testChangePinCode() passing again (please note that the Database interface has also been changed to offer the method setPin()):

public class SecurityAlarmTest extends TestCase {

  class StubDatabase implements Database {
    @Override
    public String getPin() { return "0000"; }
    @Override
    public void setPin(String pin) {
      /* Do nothing */
    }
  }

  class FakeDatabase implements Database {
    String pin = "0000";
    @Override
    public String getPin() { return pin; }
    @Override
    public void setPin(String pin) { this.pin = pin; }
  }
  ...
  public void testChangePinCode() { 
    // setup
    FakeDatabase db = new FakeDatabase();
    SecurityAlarm alarm = new SecurityAlarm(db);
    // exercise
    alarm.changePinCode("0000", "1234");
    alarm.engage("1234");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }
  ...
  public void testCanChangePinCodeMoreThanOnce() { 
    // setup
    FakeDatabase db = new FakeDatabase();
    SecurityAlarm alarm = new SecurityAlarm(db);
    // exercise
    alarm.changePinCode("0000", "1234");
    alarm.changePinCode("1234", "5678");
    alarm.engage("5678");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }
}

Spy Object

We've already seen spies in the previous post: they're useful when we're only interested in the outputs of the class being tested. They're not usually as clever as Fakes are, because they have a different responsibility: they only keep track of which methods have been invoked (and eventually their arguments); the unit test will then queries the Spy Object to assert that the expected methods have been invoked. Finally, as a courtesy, they might also return known values (fixed or configurable) to their callers but there shouldn't be any logic behind these return values. In Domus, a Spy is exactly what we need for the Logger. Reflecting the events relevant in the Home Security domain, Logger is an interface exposing the following methods:
  • alarmEngaged()
  • alarmDisengaged()
To keep this post short, I'm going to show only the code for the engagement case (you can find the complete code on git). As Database, Logger is a collaborator and we'll inject it into SecurityAlarm constructor. The requirement is very simple: when the alarm is engaged a log should be produced. The following is the unit test:

public class SecurityAlarmTest extends TestCase {
  ...
  class SpyLogger implements Logger {
    private boolean _alarmEngagedLogged;

    @Override
    public void alarmEngaged() {
      _alarmEngagedLogged = true;
    }

    public void assertAlarmEngagedLogged() {
      Assert.assertTrue("Alarm engaged not logged", _alarmEngagedLogged);
    }
      
  }

  public void testLogWhenAlarmIsEngaged() {
    // setup
    StubDatabase db = new StubDatabase();
    SpyLogger logger = new SpyLogger();
    SecurityAlarm alarm = new SecurityAlarm(db, logger);
    // exercise
    alarm.engage("0000");
    // verify
    logger.assertAlarmEngagedLogged();
  }
}

Making this test passing should be quite easy as it's just about invoking the logger when the alarm is engaged:

  public SecurityAlarm(Database db, Logger logger) {
    _db = db;
    _logger = logger;
  }
  ...
  public void engage(String pin) {
    if (isPinValid(pin)) {
      _engaged = true;
      _logger.alarmEngaged();
    }
  }

However, all the old tests fails if we pass null as a dummy logger. This is, in fact, a case when a Null Object is needed as a dummy.

Mock Object

We're not going to see this in practice because we should avoid Mock Objects as much as possible. A Mock Object is kind of a superset of a Spy. It keeps track of every single method and all its parameters that the class under test calls (even indirectly) and makes the test fail if:
  • a method has been invoked unexpectedly
  • a method has not been invoked when it should have been
  • a method has been invoked too many or too few times
  • a method has been invoked with the wrong arguments
  • a method has been invoked in the wrong order with respect to another one
All these checks might sound good but they're not in most of the cases. The reason to avoid using Mocks is because, by their nature, they impose how the class being tested should be implemented rather then what it should achieve. As soon as the class being tested is extended to implement new features (which is a very good thing), all the tests relying on Mock Objects will very probably fail (which is a very bad thing). So, what is a Mock Object good for? It's good only when we need to test that a piece of code invokes an external API properly, which is something rare and for which component tests are probably better.

Sunday, 6 October 2013

Test Driven Education [part 2 of 4]

A full Test Driven Project to learn from.

In the previous post we've seen the development cycle of TDD and a first set of its important features which I want to recall once again:
  • It leads to the cleanest and most honest API ever.
  • It leads to the right and most effective balance of bottom-up and top-down design.
  • Unit tests make refactoring enjoyable rather than risky and scary.
  • Unit tests are quick to copy-paste-tweak letting us able to test paranoid cases.
In this second post we will appreciate the effectiveness of the Dependency Injection design pattern, naturally promoted by TDD. Its name should be quite self explaining: a component relying on other objects to get its job done, ask explicitly for instances of those objects via its API, rather than locate and instantiate them on its own. Although it might sound good having a component able to locate and instantiate collaborators on its own, it is not; because it leads to Software Architecture made of components tightly coupled between each other, making automatic testing a mission almost impossible which, in turn, reduce the quality of the end product. Dependency Injection, instead, enforces decoupling and modularization, which increases the quality of the end product. The unit tests themselves prove how modular the overall architecture gets: every single component is collaborating with at least two different implementations of its collaborators: the production one and the fake one.

To have a concrete proof of this, we're going to extend our Security System making SecurityAlarm interacting with burglar sensors and sirens. However, we'll first step back to the "old" times when we used to design software up-front and then we'll compare this with TDD.

Let's have a look at a couple of requirements:
  • When the alarm is engaged, the system shall activate all the sirens when at least one burglar sensor triggers.
  • The system shall provide hot plugging of sensors and sirens.
Let's now imagine what could have happened in the "old" times. We would have probably began by observing that the first requirement is pretty easy. We can have a vector of sensors and a vector of sirens. Periodically, we poll all the sensors and if any of them has triggered, then we iterate through the vector of sirens and activate them all. Then the Chief Architect might have raised his concern about performances and might have asked to implement an EventBus to asynchronously convey sensors' signals to SecurityAlarm. The second requirement might be slightly more complicated but for sure SecurityAlarm has to open the serial ports to scan for sensors and sirens. Then the Chief Architect might have raised again his concern, this time about portability, and introduced a SensorLocator which might use the EventBus to notify SecurityAlarm about new sensors being available. A couple of meetings (and design documents) might have followed with plenty of details about the EventBus and the SensorLocator: they'll be singletons, running on their own processes, the IPC will be implemented via message passing, etc. Only after a few days we might have been able to start the actual implementation of SecurityAlarm: it might have probably retrieved the instance of EventBus first, to subscribe itself in order to receive events; EventBus might then bring up the IPC mechanism by reading some config file; SecurityAlarm might also have retrieved the instance of SensorLocator to kick off a first scan for sensors; on its side, SensorLocator might read the config files and open all the serial, USB and I2C ports accordingly. Suddenly, we would have needed a full working system and our development process would have slowed down because of it.

However, we use TDD and we can start implementing those two requirements immediately. Let's formalize the first requirement with a test:

  public void testActivateTheSirenWhenTheSensorTriggers() {
    // setup
    class SpySiren implements Siren {
      boolean activated = false;
      @Override
      public void activate() {
        activated = true;
      }
    }
    SecurityAlarm alarm = new SecurityAlarm();
    SpySiren spySiren = new SpySiren();
    Sensor sensor = new Sensor();
    alarm.addSiren(spySiren);
    alarm.addSensor(sensor);
    alarm.engage("0000");
    // exercise
    sensor.trigger();
    // verify
    assertTrue("Siren not activated", spySiren.activated);
  }
This test gives us the opportunity to see how TDD naturally balances bottom-up and top-down approaches. We've started from the bottom-up, defining how we would SecurityAlarm to look like: we would like it to have two new methods addSiren() and addSensor() to pass in a Siren and a Sensor. Going now top-down,  Siren should be an interface providing an activate() method which SecurityAlarm will invoke, when the Sensor triggers. However, since Sensor does not yet exist, we'll now jump to define its requirement (which is still top-down, by the way).

public class SensorTest extends TestCase {
  public void testNotifySubscriberWhenTriggered() {
    // setup
    class SpyTriggerListener implements Sensor.TriggerListener {
      boolean notified = false;
      @Override
      public void triggered() {
        notified = true;
      }
    }
    Sensor sensor = new Sensor();
    SpyTriggerListener spyListener = new SpyTriggerListener();
    sensor.addTriggerListener(spyListener);
    // exercise
    sensor.trigger();
    // verify
    assertTrue("Subscriber not notified", spyListener.notified);
  }
}
Please note that while writing this test we have switched back to bottom-up, defining how Sensor should look like and defining the TriggerListener interface. The following is its implementation.
public class Sensor {
  static public interface TriggerListener {
    public void triggered();
  }
  private TriggerListener _listener;
  public void addTriggerListener(TriggerListener listener) {
    _listener = listener;
  }
  public void trigger() {
    _listener.triggered();
  }
}

Now that Sensor is working, we can go back implementing SecurityAlarm.

public class SecurityAlarm implements Sensor.TriggerListener {
  ...
  private Siren _siren;
  @Override
  public void triggered() {
    _siren.activate();
  }
  public void addSensor(Sensor sensor) {
    sensor.addTriggerListener(this);
  }
  public void addSiren(Siren siren) {
    _siren = siren;
  }
}

So far, we've almost implemented both the requirements and we now have everything in place to complete them. However, we'll skip over this, as it's just a matter of applying what we've learned in the previous post.

What is really important to note is that TDD has let the design emerge on its own. Sensor can now be extended to support serial ports, USB, I2C or more as the Chief Architect was desiring but without the need of an expensive up-front design. There is also no need to poll the sensors, which is another thing the Chief Architect would have needed to design explicitly. Moreover, Siren can now have different implementations and we might even turn on a spotlight or send an SMS or more; this is something that the Chief Architect didn't predict, but TDD let it emerge spontaneously.

In conclusion, the Dependency Injection design pattern increases the modularity of the Software Architecture for free and it enforces the decoupling between components; when using TDD, it is literally impossible avoiding it!

Saturday, 5 October 2013

Test Driven Education [part 1 of 4]

A full Test Driven Project to learn from.

Since the beginning of Software Engineering we have always seen a rapid growth in technologies. Beyond all these technologies, several methodologies and practices have been developed and presented. Among all of these, only one really had revolutionized the Software Engineering: the Object Oriented Paradigm. The Functional paradigm is now coming back again, but it's not hitting the ground running, neither this time.
Today, another revolution is in progress and the world is increasingly joining it: Test Driven Development. The concept is really simple: write the test first, then write the code to make the test passing. Even if so simple, its effects are not that simple and immediate to recognize and appreciate. Its because of those effects that the TDD is a revolution in Software Engineering.

Rather than talking about TDD all in once as many other guides do, we'll apply it to a brand new educational project, detailing all its aspects on the road, while we face them. For this project, we'll develop a Security and Domotics System in Java. All the project's code is available here on bitbucket and to show the TDD cycle, I've always committed first the test and then the production code. To identify commit which adds new tests I've always prefixed their commit log with TEST:. Additionally, I've created a tag per each post. So all this post's code can be checked out from tag v0.1.

So, we want a Security System and we want it to be great! This time, though, rather than starting modeling all the components this system might be made with, we will start up our IDE and create a brand new project: Domus.

First of all Domus is a Security System, so we'll start implementing that functional area. As a Security System, we do expect to be able to engage and disengage the alarm and we want to do it by means of a PIN code. However, before engaging the alarm, we start the system up and we want it to not be engaged at this initial stage. Let's define this requirement by writing a test:
public class SecurityAlarmTest extends TestCase {
  public void testAlarmNotEngagedAtStartup() {
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    // verify
    assertFalse(alarm.isEngaged());
  }
}

This code doesn't even compile as we've not defined SecurityAlarm anywhere. However, we've defined how our API has to look like. This is the first important aspect of TDD: By writing the test first, we'll always write the cleanest and most honest API ever. Someone might argue "of course you have, it's just a stupid isEngaged() method being called". However, all of us have coped at least once with complicated API for doing silly things like this. Honesty, as well, is really important for an API. We have all faced at least once an API pretending to have less dependencies than what it indeed had, often because they were silently invoking singletons or equivalent static methods we weren't aware of.
The following is the code that satisfies the requirement and make the test passing.
public class SecurityAlarm {
  public boolean isEngaged() {
    return false;
  }
}

The next bit is engaging the alarm. To do this, we want to specify our PIN code. Again, we're now defining a new method and this code won't neither compile.
  public void testEngageAlarm() {
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    // exercise
    alarm.engage("0000");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }

This new requirement allows us to appreciate the Agile aspect of TDD. Someone would start implementing the engage() method checking that the PIN code is correct, eventually if it is well formed perhaps by means of regular expressions specified on a config file. Never do such things. Instead, if you reckon those are important features, take just note of them and stick writing the minimum amount of code that makes the test passing. After having made the test passing, review the notes and one-by-one write the requirements for those features, that is write the tests which will enforce you to implement them. Unfortunately, this requires you to be disciplined but it pays back. So, in our case, the minimum amount of code which makes the test passing is introducing a private variable tracking the engagement status and set it to true in the engaged() method.
public class SecurityAlarm {
  private boolean _engaged = false;
  public boolean isEngaged() {
    return _engaged;
  }
  public void engage(String pin) {
    _engaged = true;
  }
}

Having been now able to engage our alarm, we definitely want to disengage it. So, let's formalize the requirement:
  public void testDisengageAlarm() { 
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    alarm.engage("0000");
    // exercise
    alarm.disengage("0000");
    // verify
    assertFalse("Alarm engaged", alarm.isEngaged());
  }

This case is just the opposite of the engage() method, so we just need to set our new private variable back to false in the disengage() method. Don't be worried about doing copy-and-paste, the test is now supervising us.
public class SecurityAlarm {
  ...
  public void disengage(String pin) {
    _engaged = false;
  }
}

Obviously, the PIN code's goal is to deny engagement and disengagement of the alarm to unauthorized people. So let's write the requirements for these new features (For convenience I've put engagement and disengagement tests together here but they have actually been committed separately).
  public void testDoNotEngageAlarmIfPinIsWrong() {
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    // exercise
    alarm.engage("1234");
    // verify
    assertFalse("Alarm engaged", alarm.isEngaged());
  }
  public void testDoNotDisengageAlarmIfPinIsWrong() {
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    alarm.engage("0000");
    // exercise
    alarm.disengage("1234");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }

The minimum changes required to make these tests passing are the following:
public class SecurityAlarm {
  ...
  public void engage(String pin) {
    if (pin.equals("0000")) {
      _engaged = true; 
    }
  }
  public void disengage(String pin) {
    if (pin.equals("0000")) {
      _engaged = false;
    }
  }
}

Here comes the second important aspect of TDD: After a test have been satisfied once, it will supervise all our refactorings, ensuring we've not broken anything. This means that now we can refactor SecurityAlarm in any way we want. If the unit tests pass, then we've got it right. In our case we can remove the string literals and introduce a helper private method isPinValid() to verify whether the PIN code is valid or not, so to increase code expressiveness.
This is a silly refactoring right? SecurityAlarm is the only class in this tiny code base… It has only three methods… However, the first time I've done this refactoring it was almost 2am and I've got it wrong and the unit tests caught my mistake. Immediately! I've also committed my error on git so that you can check what I've missed. Basically, with that bug anyone would have been able to engage/disengage the alarm even with the wrong PIN code! Try imaging how boring and disappointing it would have been to boot the system up, engage the alarm (with the right PIN), type then the wrong PIN and discover that the alarm is disengaged!
Here is how SystemAlarm looks like after the refactoring:
public class SecurityAlarm {

  static private final String DEFAULT_PIN = "0000";

  private boolean _engaged = false;

  private boolean isPinValid(String pin) {
    return pin.equals(DEFAULT_PIN);
  }

  public boolean isEngaged() {
    return _engaged;
  }

  public void engage(String pin) {
    if (isPinValid(pin)) {
      _engaged = true; 
    }
  }

  public void disengage(String pin) {
    if (isPinValid(pin)) {
      _engaged = false;
    }
  }
}

As customers, we would be very disappointed if we couldn't change the PIN code. Let's put down the requirement. We want to change the PIN code but, to be sure that we're the only one who can change it, we want to input also the old PIN code:
  public void testChangePinCode() { 
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    // exercise
    alarm.changePinCode("0000", "1234");
    alarm.engage("1234");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }

The minimum amount of changes that make all the unit test passing consist in introducing a new private variable to store the PIN and defaulting it to the factory PIN:
public class SecurityAlarm {
  ...
  private String _pin = DEFAULT_PIN;
  private boolean isPinValid(String pin) {
    return pin.equals(_pin);
  }
  ...
  public void changePinCode(String oldPin, String newPin) {
    _pin = newPin;
  }
}

However, the feature is not complete yet. We want to make sure that only who knows the PIN can change the PIN:
  public void testDoNotChangePinCodeIfOldOneIsWrong() { 
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    // exercise
    alarm.changePinCode("1234", "5678");
    alarm.engage("5678");
    // verify
    assertFalse("Alarm engaged", alarm.isEngaged());
  }

This is the working code:
  public void changePinCode(String oldPin, String newPin) {
    if (isPinValid(oldPin)) {
      _pin = newPin; 
    }
  }

Here comes another important aspect of TDD: Since unit tests are tiny, it does cost nothing to copy-paste-tweak an existing test to test some edge or paranoid condition. In our case. We might want to test if the Security System allows us to change PIN code more than once and it does really validate against the old PIN code rather than the factory one. Sounds paranoid? In this case it's probably not, but even if it was copy-paste-tweak of 5 lines would cost less than 1 minute of "typing".
  public void testCanChangePinCodeMoreThanOnce() { 
    // setup
    SecurityAlarm alarm = new SecurityAlarm();
    // exercise
    alarm.changePinCode("0000", "1234");
    alarm.changePinCode("1234", "5678");
    alarm.engage("5678");
    // verify
    assertTrue("Alarm not engaged", alarm.isEngaged());
  }

This test passes, proving that we got it right the first time, but we now have another test supervising us.

This concludes the first part of this educational project and we've already implemented important features in the overall system. It's worth to highlight that to implement this central features we didn't spend any time designing any component. This is because TDD is the most Agile methodology, as it promotes the design to emerge on its own, in a bottom-up fashion, rather then in the classical top-down one of modeling several components and their collaborators up-front and narrowing down their details. However, TDD is not purely bottom-up, but it balances itself with top-down as we first identify the component to assign a given responsibility to (top-down) and then we implement it by increasingly extending its requirements and introducing its collaborators (bottom-up).

Before leaving, it's worth to briefly recall the important aspects (or "features") of the Test Driven Development we've seen so far:
  • It leads to the cleanest and most honest API ever.
  • It leads to the right and most effective balance of bottom-up and top-down designing.
  • Unit tests make refactoring enjoyable rather than risky and scary.
  • Unit tests are quick to copy-paste-tweak letting us able to test paranoid cases.