I’ve managed to complete my other project within my original estimates – I will blog about whether visualising the work and the other approaches I took worked/helped next time!
Today’s task has been to start converting the UI layer of the main legacy system that our team maintains to MVP. I’m not going to delve in to the reasons why we chose MVP; well I think we just decided that it would be the easiest path to getting the UI under test. The team probably has more experience with MVP than MVC so it’s probably a wise choice!
I’ve taken a relatively simple screen to start with, one that shows the notes on a specific Sales Order, allows some filtering, and allows the user to add an additional note. Instead of delving straight in to the ASP.Net code and slotting tests around the existing functionality, I chose to simply abstract the current functionality in to the presenter. This was all done using tests to drive the code. Essentially the tests are what the code does “as-is”, so I suppose you could say that I let the current implementation drive the tests to a certain degree.
As I started to increase the number of tests around my newly created notes presenter class, interesting patterns and problems with the existing code emerged. Since I’ve become an advocate of a Test-First approach, I’ve noticed that code not written by tests doesn’t show problems – early in the development cycle. I also think (maybe wrongly) that code doesn’t get refactored if you don’t have good tests – therefore ending up spaghetti-fied or just damn hard to highlight problems.
The existing code does a number of things:
- Shows all the notes; Or;
- Allows you to filter notes
- Then displays the count of each of the types of notes
Now as the tests have grown I’ve found the existing code does the following:
- Shows filtered notes/all notes
- Then gets all the notes again to display the count of each of the types of notes.
The presenter should just get all the notes then filter based on the current filter criteria. This removes the dependency on multiple calls for the notes.
The other thing I found was that the NHibernate ISession is used at quite a high level – with the Service (essentially the Model that I’ve been using for the Notes – it was in existence already) that I am calling to get the Notes. The problem was that I had to force the ISession in from the Presenter. This has made me feel really uncomfortable –a code smell! I’ve had to mock the ISession object out for all of the tests since I am using constructor injection. Really this dependency should be pushed right in to a Model.
Thinking about it further (as I write this blog post) – it is probably about time to create a real model. I thought I could have got away with using the existing note service for this purpose. This in essence would be a model/façade so I no longer have to mock the NHibernate ISession.
This is something I will endeavour to do on the next phase of development (i.e. tomorrow!).
I find it interesting that the things above highlight themselves after only writing a dozen or so tests. It definitely backs up my thoughts on test-first development, and makes it even more crucial to write them all the time. There has also been the added benefit that I’ve been able to fix a minor UI bug with the code that I’ve written.