Friday, January 23, 2009

The birthday greetings kata

On February the 14th the milano-xpug organized a kata whose subject was proposed by Matteo on his blog to introduce the exagonal architecture.

Unluckily I couldn't attend the meeting in the flesh, so I just decided to try the kata out here on my own. I first started from scratch, thinking it would prove more fun; then, as I was taking quite a different approach - you could also say that a whole different design was emerging - I decided to get back and start from the original code provided and refactor it to try and focus on the architectural issues which were the aim of the exercise.

Even if I think this exercise to be quite simple, I decided to post a trace anyway because someone might still find it useful. You can never say!

The heart of the application is the BirthdayService class, whose responsibilities consist in identifying employees whose birthday falls on a given date and sending them a greeting message.

This is the public method, called by the acceptance test:


I know it looks odd, but I've had enough of escaping characters and trying to format code to make it look nice, so I hope you will forgive me.

Anyway, the sendGreetings method parses an input file, creates employees, checks if they are eligible for a greeting message and delegate the actual sending of the message to a private method:


Seems a great bunch of things for a single class... let's try to sort it out. First I decoupled the service from the file with the employees' names introducing the interface EmployeeRepository, with the method getEmployees, then I wrote a FileSystemEmployeeRepository that implemented it. The repository was then injected into the BirthdayService via constructor by
the acceptance test. At this point the code looked like this:
...
List<Employee> employees = repository.getEmployees();
for (Employee employee : employees) {
if (employee.isBirthday) {
//compose message and call method that sends it
}
}
...
Still I thought that the check on the date of birth could be done by the repository itself, so I changed the interface adding the new findEmployeesBornOn(OurDate) method and changed the code so that it looked like this:
...
List<Employee> employees =
repository.findEmployeesBornOn(ourDate);
for (Employee employee : employees) {
//compose message and call method that sends it
}
...
Maybe not a very big change but I think it was worth it, as now the code is cleaner. Then there was the messaging part. First of all I decided to introduce a new BirthdayGreeting class, that produces a message body, a fixed subject and a recipient starting from an employee; at this point I extracted the interface SimpleMessage that exposes these three strings. The code had become
...
if (employee.isBirthday) {
SimpleMessage greeting = new BirthdayGreeting(employee);
sendMessage(smptHost, smptPort, "sender@here.com", greeting);
}
...
Still I had to resolve the dependency from the SMTP, so just like I did for the repository I introduced the MessagingService interface with the sendMessage(String sender, SimpleMessage message) method and injected it into the BirthdayService via constructor from the acceptance test. Some nip and tuck to move the body of the sendMessage method from BirthdayService into a new EmailService, implementer of MessagingService, and voila, the new BirthdayService code:
public void sendGreetings(int month, int day) {
List<Employee> employees =
repository.findEmployeesBornOn(month, day);
for (Employee employee : employees) {
BirthdayGreeting greeting = new BirthdayGreeting(employee);
messagingService.sendMessage("sender@here.com", greeting);
}
}
I also changed the method parameter from OurDate to month, day. Now we have a simple and decoupled service that can focus on its true responsibilities: ask for all people who should receive a greeting, create a new message and delegate its sending.

One might wonder whether the sender belongs where I put it, or if the service should directly instantiate a BirthdayGreeting with a new rather than getting it in some other way. But maybe that's going too far.

BTW I also decided not to use the Employee.isBirthday() method and I created a value object called IsBirthday, created with two parameters representing month and day, with the boolean method forEmployee(Employee). The code in the repository now looks like this:
public List findEmployeesBornOn(int month, int day) {
List<Employee> list = new ArrayList<Employee>();
IsBirthday isBirthday = new IsBirthday(month, day);

for (Employee employee : employees) {
if (isBirthday.forEmployee(employee)) {
list.add(employee);
}
}
return Collections.unmodifiableList(list);
}
That's all folks... maybe I'll manage to post the complete code here, where you can find some other (and more accredited) solutions produced by people who actually were there.

No comments: