BDD with Cucumber School

I have been a BDD practitioner for a few years, a combination of self-learning and attending various user groups and getting useful feedback from my peers.  BDD is something you pick up gradually and gets better with practice. My personal experience when starting to use BDD was that it can be a form of information overload because one is not sure where to start. With newcomers, sometimes the focus is on the tools rather than the extracting and understand the actual behaviour of the business. Some people think they've practised BDD if they just write scenarios and automate their examples, a tick box exercise. But BDD is and should be larger than that.

I was fortunate to be asked to peer review a set of online training videos from the Cucumber School, the team who originally created Cucumber. The course assumes no prior knowledge in BDD concepts or the use of Cucumber. Every video has a range of exercises that you can perform after watching to reinforce your learning.  This is great as it means that the videos are not a dry exercise in memory retention but rather an impetus for self-learning.

I was gratified to see that the initial lesson were geared towards fleshing out behaviour via conversations or examples rather than diving straight into the tool. This is achieved by the need for distinct user roles in defining and refining those examples.

A team is usually built of different disciplines such as developers, testers and a business representative such as a product owner. What was hammered home during the lessons was the need for continual collaboration  and communication between all team members so that the specifications constructed were effective and garnered common understanding.

An exploration of the Gherkin syntax was covered. Gherkin is a business readable DSL that describes the behaviour of your software without detailing its implementation. You can think of this as a form of standardization in how your scenarios should be constructed. Cucumber School gave an excellent explanation on Gherkin and how to tailor scenarios effectively.



Later lessons then dived into the actual Cucumber tool itself, right from setting up the IDE, generating skeleton code from scenarios and then fleshing out the implementation. This was all performed in an iterative manner.  I found this extremely beneficial because it demonstrated how to build up the executable part of a specification gradually.  The code was incrementally built to support the individual steps of a Gherkin scenario. The re-use of step code across similar steps was a fundamental point through the judicious use of regular expressions. Again no former knowledge of regular expressions was expected.  A beginner could still pick up the salient points and write their own expressions by the end of the lesson.  Emphasis was also placed on the readability of scenarios so that they were kept relevant to the business and their intent was clear.

When devising steps, another point conveyed that the steps should really be a thin veneer across a domain model. This is important because the domain model should be able to exist independently of the BDD framework. TDD was heavily enforced to flesh out the model that the scenarios suggested. Constant refinement of scenarios and unit tests was encouraged. A great point was made about listening to tests because they were a reflection on how well the domain was understood and also on how well that domain or scenario could be tested.  Removal of whole scenarios was also encouraged if those scenarios no longer had any value.  A scenario may have been written which in retrospect duplicated  or was a simply a generalization of another scenario which captured that behavior more succinctly. In that case, the scenario should be pruned.  Refactoring of code but also refactoring of the scenarios themselves was a salient aspect of the training. For instance, some scenarios were better described with the use of tables. This was all to keep the scenarios understandable and focused. Another point that was driven home was to strive to get the scenarios described at the right level of detail. Enough for the feature to be adequately described but not so little that the scenario became too vague or not too much detail so that the implementation did seep into the scenario.


A practice which I intend to follow is Example Mapping. This is  a great tool during the inception phase of a project when one is trying to understand the problem to solve whilst also taking note of those questions we still have to answer. Example Mapping  is a technique to break stories into rules and examples. This is a great example of Deliberate Discovery where we try to get feedback for the application we're about to build before we actually write any code. By focusing on examples, we get a feel on how the application will behave before its existence.

I like that a holistic view of development was portrayed in the sense that BDD and TDD were not considered disparate, but practices to be followed at different levels of granularity. A great phrase that resonated with me was that BDD enforced outside-in development.

BDD ensures you're building the right thing, but TDD ensures you're building the thing right. 

Also emphasized that BDD encourages different types of testing should be covered at different levels.  For instance, there should be a plethora of TDD tests at the bottom of the scale, then system or integration tests to tie the domains together with BDD or scenario level tests above that.   At the top should exist exploratory testing. What sometimes happens is that although automation is in place, there is still a substantial amount manual testing undertook, which is both laborious, slow to complete and error prone.

BDD encourages that the higher up the pyramid one goes, the less testing there should be. All of the testing permutations happen lower down so that as we rise, although the tests become fewer, they become more important.

The final lessons touched upon testing the domain model from a web-front-end.  Automation was achieved via the use of Selenium.  I'm not a front-end developer, but the example shown was easily understandable and a great base to build upon for future front-end testing.

It was great to see the hexagonal design pattern being promoted to facilitate easy swapping of testing the domain directly or from the front-end.  This pattern also known as a port and adapters allows one to separate the core business logic from anything that touches the outside world such as front-ends or databases. This enables the domain to be tested either directly or from a browser. It was a great example of front end testing but also defining scenarios so that the domain was separate from any front-end concerns.

(Reprinted from Nat Pryce's Ports and Adapters Architecture)

As I proceeded through the lessons, any questions I had on BDD on testing or the right way to approach devising a domain model were answered by later lessons.  This was a great way to learn and for those points to stick in the mind.

A lot of people still think BDD is a tool and is only about testing. For a long time, I was one of those people too. I believe BDD is still poorly understood and frequently misappropriated. But following the Cucumber School lessons, I think a lot of the misnomers of BDD will be eradicated. At the end of the course I feel I've gained several approaches to finding solutions to problems in new ways.

So in conclusion,

• The videos and related exercises are excellent in describing the benefits of developing software following a BDD approach
• I liked that the modus operandi was to adopt a reactive approach rather than trying to get everything correct the first time round.  Recognizing shortcomings in scenarios or code and test smells led to feedback loops in which those were corrected later on. I think this was a great point to take away and remember.
• I liked how BDD was presented as complimentary to other practices or development techniques such as TDD or DDD. 
• The lessons are very good for newcomers but experienced practitioners will also find useful information. I've certainly learnt some practices I wasn't aware of, which I'll carry into my daily work. The series of lessons from Cucumber School come highly recommended.

Code as you would order a burger

Recently during my work, I came across some code where the paradigm Tell Don't Ask came into play. You sometimes see code written that's not really using OO to its full strengths and still is of a procedural stance. There's often frequent examples where an object asks other objects for data and then does something with it. This is not really OO even though you may still be using other aspects of OO such as encapsulation, inheritance or polymorphism. It's still asking for data whereas the real value-add for OO is message passing or the behaviour of the objects rather than the data they may contain. I've come to understand this implicitly via the usage of mocks during testing. The use of mocks results in code that better follows the 'Tell, Don't Ask paradigm. One of the originators of mock objects noted that:

The trigger for our original discovery of the technique was when John Nolan set the challenge of writing code without getters. Getters expose implementation, which increases coupling between objects and allows responsibilities to be left in the wrong module. Avoiding getters forces an emphasis on object behaviour, rather than state, which is one of the characteristics of Responsibility- Driven Design - Tim McKinnon

Once I came over this mental hurdle, I found I wasn't primarily concerned with the data passed between the objects but really that the appropriate methods calls or methods (messages) between objects were observed. I used the Mockito mocking library and I find the most of my tests are interaction tests i.e. verifying that methods calls were made in the manner expected. State-based testing is still important but at the right place. This got me thinking about objects in terms of Alan Kay's emphasis on message passing http://c2.com/cgi/wiki?AlanKaysDefinitionOfObjectOriented. The following examples illustrate why telling an object to do something rather than asking it for its data is better.

I work a lot with enterprise integration and one of the examples that illustrates this point is message concatenation. During conversion from an internal message to one that that is supported by an external protocol. Dependent on whether a message is concatenated or not, then the message may be decorated with extra parameters to indicate that it is a multipart message rather than a single message.

In this example, the extra metadata parameters that get added are:

• The message reference for the multipart message
• The part number of the message
• Total number of parts for the message

Implementation using "Ask" approach
A factory was used to create a map of parameters for message concatenation

Loading ....

The protocol converter that used this information was coded like this:

Loading ....

This bad for several reasons:

• The MultipartMessageParameterFactory returns an abstract datatype. In this case a map, so users of this class have to know what keys to pull out the data. If this approach really must be followed, a domain object to hold the multipart parameters would be better, maybe a simple POJO with accessors for the different parameters.
• The main problem with this approach is it results in Inappropriate Intimacy. The user is asking for some data and then assembling that data thereafter. The lookup keys for the map are made public presumably to mistakenly enforce DRY principles for magic strings within a unit test.
• This class could become better by passing in the parameter factory in the constructor so it can be mocked. In this way, the unit test for this class will be simpler although it remains too complicated.

Implementation using "Tell" approach

The Tell is all about telling an object to perform some piece of work on your behalf rather than asking that object for data and then doing the work yourself. In the previous example, the protocol converter asked another object for the message concatenation info and then added those parts itself to the message. In the following example, we ask another object to do this bit of work for us. Firstly we introduce a domain object to replace the map use for the multipart parameters:

Loading ....

The factory now returns this POJO instead of the naked ADT:

Loading ....

Now the converter asks another object, MultiPartMessageUpdater, to do the message updating:

Loading ....

The code for updating the message is pushed into its own class. As well as enforcing the single responsibility principle, it also endorses the command and query separation principle so that the retrieval of the actual parameters (query) is separated from the addition of those parameters to the message itself (command).

Loading ....

As a byproduct, the test for the converter becomes very simple. Beforehand we would have to test every permutation of concatenated parameters. Now because code has been pushed to their logical place the test boils down to just testing the following behaviour:

• Message should be updated if message is part of a multipart message
• Message should be untouched if message is just a single message

Loading ....

The permutations for those 3 message parameters are pushed to where it should be namely the test for the MultipartMessageParameterFactory. Each test then tests what it should instead of testing the unit and all of its dependencies' behaviour at the same time resulting in a combinatorial explosion of test methods making the test unclear.

Conclusion

• We've now got a class which now asks other classes do work i.e. operation requests instead of asking for data and doing the work ourselves.
• By following this approach the single responsibility principle naturally falls out. Each class performs a single well, understood operation and only that operation. This makes those classes loosely coupled and become candidates for reuse in different contexts if needed.
• Unit tests have become clearer and focused as they only testing a single object rather than a multitude. If you notice I've endeavoured to use constructor injection for any dependencies. This makes the testing of classes easier as I can mock out any external dependencies.

I remember this paradigm by thinking what you would do if you ordered a burger in McDonalds. Would you ask the server for two sliced bun halves, some mayo, lettuce, a hamburger, cheese and tomato sauce or would you just ask for a Big Mac? :D
Links

• Mock Roles not objects by Tim Mckinnon, Joe Walnes, Steve Freeman and Nat Pryce
• Alan Kay's message to the Squeak mailing list about OOP, inheritance, and message passing.
• Growing Object-Oriented Software by Steve Freeman and Nat Pryce

Constructors that do too much

A flaw I've notice frequently in a legacy code base is constructors which are doing too much work. A common anti-pattern is that a configuration is passed into a constructor. This configuration is then perused to find information so that further objects can be constructed in the constructor. I will discuss why I think this is bad practice.

As an example I have a message dispatcher which delegates to a sender object
to send a message to a destination. A dispatcher usually makes a decision on the appropriate destination based on the message. For this example there is only one endpoint. This destination is to be read from the configuration.

So I have an interface for the sender:


and a dispatcher which uses the configuration to build a sender inside its constructor:


The expected usage for the constructor then becomes:

This is bad on several counts:

• Abstract data type is passed in. There's no indication how this will be used or what it's used for. It's just a bucket of parameters. If possible a specific type should be used in preference to an ADT. It's all about the domain and context. Check out Domain Driven Design  or GOOS touches on this point.
• Harder to unit test. I have to make sure configuration is correct so that internal objects can be created properly. In my example I've only one object to consider but they may be several. 
• I can't mock the behaviour of any objects created by the constructor. In essence that behaviour is hard-wired which may be deleterious to testing if that behaviour is using a real resource i.e. socket or database. It turns the unit test into more of an integration test

Use a factory approach

So instead of passing in a configuration, pass in the already constructed objects. A DI framework such as Spring or Guice will assist you in this regard as it's fundamentally what DI is all about. You inject pre-configured objects into other objects rather than building those objects internally.

If you're not using a DI framework (I'm in this situation as it's legacy code) then it's better to pass a factory which builds the object on the fly as a constructor parameter.

To illustrate this:

I can have a factory that produces Sender objects. I've taken the liberty of introducing a generic factory for this purpose. It builds objects of the specified type for a given configuration.



Now I provide an implementation for the Sender:

And to illustrate the point I rewrite the Sender class:


Finally I rewrite the message dispatcher. The constructor now takes objects rather than configuration


And the usage becomes:



This is a lot better:

• Now the dispatcher only knows about objects. It does not even know a configuration is used.
• The transformation of a configuration into object is localized in the factory and not spread out across the code base
• The class becomes trivial to unit test. Instead of worrying about configurations I can simply create a mock Sender and pass that to the dispatcher. In this way I can focus solely on the behaviour of the Dispatcher itself.

Conclusion

Resist the urge to pass around configuration objects and allowing constructors to build new objects. A constructor should ideally be used for assembly only. It's better to just pass pre-configured objects into a constructor. This makes the intention of that constructor clear and makes it easier to unit test the class in question as the colloborators can be mocked easily.

JUnit Theories to the Rescue

I recently developed a restart manager to respond to JMS connection outages. After the connection was restored, I needed to reinitialize my message clients in order for them to rebuild their JMS sessions. But I needed to test that a restart only occurred for particular status transitions. At first I thought this would be an onerous task as I would have to work out all the status permutations. That is until I came across JUnit Theories.

JUnit Theories
Theories allows one to write tests that apply to a (potentially infinite) set of data points rather than having to recreate the same test multiple times with different data or creating one test and iterating through your own collection of data values.
Here is an example:

Messaging client
First off is the interface for the messaging client. It has two lifecycle methods, start and stop which are self-explanatory, as well as methods for reading and sending a message which for brevity are just strings.
Status Enum
The following enum represents the status of the JMS connection. As can be seen there are numerous states so it would be cumbersome to work out all the possible combinations for each status transition. This is where Theories become so useful. More of that later.

Restart Manager
This class is used to restart a message client when it recognizes a status transition from FAILED to STARTED.
Test class
Finally here's the test class. Notice I'm using mock objects here using Mockito. As I'm programming to the interface of the messaging client not its implementation, I can make use of a mock and verify its behaviour.  Presently as of JUnit 4.8, theories is still within an experimental package.
The most important bit of configuration is @Datapoints annotation. This sets up the data that'll be pass into the test methods. In this test I have two theory methods. One theory checks that the message client is restarted when there is a suitable status transition i.e. FAILED to STARTED. Another theory checks that the message client is not restarted if the status transition is not suitable. i.e. not FAILED to STARTED.
When you run the tests you'll find the all the combinations of the Status enums are passed to the test methods in question as shown below:
In this way, I ensure that all possible transitions are covered by my unit tests and the restart manager's behaviour is as required.
Conclusion
The use of theories allows tests to be devised that cover all possible combinations of data. This is in contrast to parameterized tests where the dataset to be passed to a test is strictly defined and the onus is on the developer to work out what data is needed for a particular range of tests. Each approach can be used in different situations as dictated by your requirements.

Programming Retrospective

I've recently had to develop and improve a legacy code base. This has reminded me of a several anti-patterns one should consider when coding. This is by no-means an exhaustive list but conveys some of the most frequent ones in my experience.

Final classes without interfaces

There were a lot of instances where classes were declared as final. This may be a good thing from a security perspective but from a testing perspective it meant that mock classes could not be created resulting in unit tests that were harder to write or could not be written.

There are tools to can create mock objects even without an interface. A particular favourite of mine is Mockito. In fact the mocking of final classes can be overcome through the use of the PowerMock API with Mockito.  However a class with an interface is preferable from an OO standpoint as the use of concrete classes results in tighter coupling between participants.

Lack of Defensive Programming

There are numerous examples of methods without any defensive checks. The caller of this method had a try catch block to catch a NullPointerException, NPE. This is a bad code smell because the onus is on the caller of the method to assert the validity of parameters that are passed to the method in question. This is the wrong place. From an encapsulation point of view, it should be the method that validates whether its input parameters are valid or not. This is a form of Design by Contract where the method checks to see if input parameters satisfies some condition and reacts accordingly. I like to think this as the 'Bouncer' Pattern. If you don't look right, you're not getting in :).

A contrived example is shown below:


This short of approach should also be applied to constructors to ensure the object has a valid state and is fully built before use. Furthermore the throwing of exceptions give more context as to what has caused the failure rather than catching a NPE and having to retrospectively determine what has called that NPE to be thrown.

A favourite API of mine is the Validator class in Commons Lang which lists numerous methods for validation in different contexts.

Exposure of super state to child classes

I've seen a lot of instances where child classes would use the state variables of a super class. Variables had protected visibility so there was naked access to these variables which are inherently dangerous. This is because a child class could change the reference to a super class variable with unforeseen consequences.  I think the intention was to provide access to a child class with a parent's state so the child class can perform an operation. A quick fix would be to only allow these parent state variables to be access through accessor methods.

http://c2.com/cgi/wiki?InappropriateIntimacy


But this begs the question on why child classes needed that sort of access in the first place. IMHO, private state should never escape the confines of a class, only behaviour.

The other smell was that the super class was becoming top heavy with functionality. Probably the reason was that common functionality was pushed up the class hierarchy for re-use by child classes. But this resulted in the super class becoming bloated and unfocused. A better solution would be to use delegation techniques rather than inheritance.

http://www.refactoring.com/catalog/replaceInheritanceWithDelegation.html

When I look at a class functionality, I like to keep in mind a Unix philosophy i.e. Do one thing but do it well. If you find your class not adhering to that maxim, that's a sign refactoring is in order.

Printing out error messages to console instead of logging

I've came across a few situations where exception stack traces were dumped to console. Don't do that. Use a logging framework such as log4j. If log4j is used then a console appender can be used to achieve the same goal. Furthermore errors and warnings could be logged to a specific destination i.e. a file so one can see only pertinent errors and not worry about debug messages.

Another observation was the lack of categories used in logging. Most of the statements I saw were a generic dump of error messages for the whole platform. Using categories allows log messages to be sorted. For example I could have a category called com.acme.X for X related logs and com.acme.Y for Y related logs. If I wanted to see all logs I create another appender that logged at the com.acme level.  At the very least, use the fully qualified name of the class the logger resides in as the logging category. The use of categories results in greater capabilities on what should be logged and where it should be logged to. In this example X and Y are logged to different appenders but the possibilities are endless depending on how the categories are devised.

Classes with unclear focus

There were examples of classes trying to do too much. An example would be a Handler class. The main functions are listed below

• Setup relevant properties needed by the handler
• Handle requests.
• Convert a request to a protocol specific message.
• Handle synchronous and asynchronous responses

This resulted in a class of over 1000 lines with several private methods.  This was one of the major smells in that many private methods were only working on a subset of the private fields of the class. This implies that the class is trying to do too much.

Most of the examples I've seen are that the majority of functionality is realized inside one class instead of being delegated to other classes. The lack of delegation means the intention of the class is lost. Furthermore testing of the handler becomes more problematic. By delegation, each of these functions can be tested in isolation.

http://c2.com/cgi/wiki?LongMethodSmell
http://c2.com/cgi/wiki?GodClass

Unwieldy or unneeded comments

There were a lot of instances where code comments were of no use or didn't add extra information. For example, one method contained a lot of retrievals from a database along with ambiguous looping constructs. Each part of the method contained a comment explain what the next section of code would do. The reason I don't like this firstly comments are deodorant on 'smelly' code. That comment is probably there because the code is not clear enough to be understood. Secondly comments are brittle. If I changed that section of code, then I have to remember to change the comments, another piece of maintenance.

I am a proponent of 'Programming by Intention'. This is a programming style where you give meaningful names to methods, variables, classes etc so that the intent of the object in question is clear. Dave Astel gives an excellent overview here: http://www.informit.com/articles/article.aspx?p=357688

In the case of the Handler class, the method was essentially doing three things:

• Obtaining a customer ID
• Obtaining an billing ID
• Obtaining other parameters from a database and checking to see if those parameters had values.

This resulted in a method of  200+ lines. The clarity or intention of the method has been lost. To regain the intention of this method, the method should be functionally decomposed into smaller methods.

i.e.


Now the intention of this method is clearer. The code becomes self-describing and there is not need for extraneous comments.

'Programming by Intention' is not used to declare all commenting is bad, just that commenting must not duplicate a purpose. If the code is clear then commenting what the code does is unneeded. However comments may still be needed. You could draw attention to a particular algorithm being used i.e MergeSort or that the code fixes a particular defect. When the comment has value it should be included, if not it should be discarded.

Use of exceptions to control program flow

Simple. Don't to it. The following link provides arguments:

http://c2.com/cgi/wiki?DontUseExceptionsForFlowControl

Throwing of ambiguous exceptions

There were numerous occasions where java.lang.Exception was thrown instead of a more specific exception. This is bad practice because throwing an ambiguous exception means the catcher cannot react to the exception in different ways. An ambiguous exception loses information on whether the situation is recoverable or irrecoverable. The meaning of the error is also lost. A specific exception should be thrown for a particular situation.

http://c2.com/cgi/wiki?ExceptionPatterns

Use parameter objects instead of long method signatures.

There were a few cases where methods had long method signatures. I'm talking about 10 or more parameters. This makes the method call unwieldy and prone to mistakes. A better approach is to use a parameter object which encapsulates the method signature, simplifying the method call.

http://c2.com/cgi/wiki?ParameterObject

Furthermore different parameter objects can be used to group together related parameters for different contexts. This is preferable than nullifying unneeded parameters in the long method signature.

Never Duplicate Code

http://c2.com/cgi/wiki?OnceAndOnlyOnce

A few examples were observed where code was duplicated and in some places just a copy and paste job.  This means that if any defects are found, then the fixes have to be applied in more than one place. Ideally code should be written using DRY principles i.e. Don't Repeat Yourself. Situations where the same code exists in different places should be remedied by that code being pulled out into a separate method and re-used.

Return nulls from methods.

There were a few cases where a call to a method resulted in a null being returned. For instance a client asked for a map and got returned a null because the input parameters were incorrect. The onus is then on the callee to check the result is not null before using the result. A better approach would be to return a NULL object. The Null Object pattern provides an alternative. It connotes the absence of an object. Instead of using null, the Null Object pattern uses a reference to an object that doesn’t do anything.

http://en.wikipedia.org/wiki/Null_Object_pattern

In this example, instead of returning null, an empty map should be returned. The client then doesn't have to check for nulls. This leads to safer code.

Conclusion

A lot of my recommendations are based on Martin Fowlers' Refactoring which gives guidance on how to remove particular code smells. However as legacy code is usually not particularly amenable to unit testing, refactoring can give a low confidence level as there are not the number of unit tests to back it up. Part of the Test Driven Design (TDD) approach is that unit tests are written to prove the behaviour of the system at a granular level. Once you have the tests, you have the confidence to refactor as you can regression test to see the system behaves as before. In my opinion a lot of the defects seen in production code would be diminished by the use of unit testing and paying heed to the aforementioned anti-patterns.

Recommended Reading

• Refactoring: Improving the Design of Existing Code by Martin Fowler. The classic book on taking poorly designed code and making it better without breaking it.
• A good introduction to TDD. Test-Driven Development: A Practical Guide by Dave Astels
• A TDD book for the more seasoned developer: Growing Object-Orientated Software, Guided by Tests by Steve Freeman and Nat Pryce. Probably if TDD was adopted in the first place many of these problems I described would not have arisen in the first place.
• But if you have a legacy code base showing these symptoms then a great book to read is Working Effectively with Legacy Code by Martin Feather. It has a set of techniques for changing code so that it is more suitable to be covered by unit tests