Conflict in the Workplace

I'm doing some work for a strange little company. Its an unlikely assortment of folk from vastly different backgrounds. I've been trying to hammer home the importance of using what I consider to be best practices. I lean towards an agile style using test driven development, continuous integration and pair programming as well as lightweight UML for working out designs.

There was a section of the codebase which kept turning up as the source of bugs. It was a model in the MVC sense that was statically accessed through the session. Around half of the UI depended on this object but these various components expected it to behave in a different way. Essentially it was the core data that everything needed, but the view of it that they required differed. A set of clunky switches had evolved to prime the model to behave in a certain way. The bugs centered around the fact that the model was often left in an inconsistent state because it wasn't clear exactly how it should be poked and prodded to achieve the desired state.

It seemed clear to me that we needed to refactor. Firstly I realised we needed to have subclasses of our 'god-object' model to separate out the concerns that were proving difficult to manage from a single class. Secondly it was difficult to write tests against objects that depended upon the session. There was a great deal of test friction involved in setting up a mock session and our tests had to effectively start the application in order to run, just to instantiate the model object we were trying to test.

The model was accessed via a call to Model.get(), a static accessor that retrieved the instance from the session. I wanted to replace this with an actual instance obtained from a factory or provider. I also wanted any classes to depend upon an interface for the model, not the actual class.

In my first cut I created some subclasses of our model and moved the bits of implementation out of the existing lengthy if statement ridden implementation into appropriate classes. These classes still depended upon the static accessor, but at least we now had a facade that could give us classes that were concerned with discrete chunks of functionality.

The second job was to remove the static access. This proved more difficult than I thought. This was because rather than model dependent classes being passed their model in the constructor (we're using wicket) they were sometimes using the static accessor to obtain a different model from that which had been used during construction.

The aim was to make everything dependent on an interface which was extracted from the existing model. With static access removed we were starting to have classes that became more testable. By refactoring to use interfaces we were able to provide an alternative implementation. Dependent classes wouldn't care which implementation they were using as the interface hadn't changed.

Now, the title of this post is 'conflict in the workplace', and here's the thing. Last week I was pairing on this refactor, working through our application and providing new implementations. It was going well. I had two days holiday and when I came back on Monday I discovered that the developer I had been pairing with had dramatically changed the interfaces.

When I began this refactor I had planned out exactly what I thought we needed to do and presented it to the team. I had their support and welcomed their suggestions. This seemed to be in stark contrast to a developer making sweeping changes in my absence with no consultation.

I had been refactoring to provide a less bug prone implementation of our existing API. Suddenly someone had gone rogue and changed the API. Since these changes I've been working through the application updating everything that depended on the old API to work with the new one. Not everything that is required is in the new API so I find myself rewriting code that had been working to fit the new order.

During a heated debate yesterday, I complained that I felt I was now wasting my time fixing code that turned out to not be needed any more. His response was that it was because I hadn't bothered to learn the new API. It was at this point that I realised I was angry that the API had changed. To me it was important not to break compatibility of our interfaces. To do so had far reaching implications on the rest of the application. During all of our discussions we'd never decided the API was wrong. Now I was fixing the fallout which was the result of new interfaces.

I'm not against these sort of changes, there are always different ways of doing things. I did find it very difficult to accept that such vast changes were made by an individual in my absence with no discussion.

I'm not about to dictate that we rollback everything that has been done. People tend to be unhappy when they are told what to do. We will be able to fix things. Sometimes people need to make mistakes in order to learn. And the lessons learned this time I hope are that buggy implementations should be replaced by better implementations. Interface compatibility should not be broken. And decisions affecting the whole team should be taken by the whole team. Rant over.