Mark Needham

I came across an interesting set of posts by Jeff Wofford where he talks about programmers getting stuck and it made me think that, despite its faults, agile software development does have some useful practices for stopping us getting stuck for too long.

Many of the examples that Jeff describes sound like yak shaving to me which is part of what makes programming fun but doesn’t always correlate to adding value to the product that you’re building.

Although I wrote about some of the disadvantages of pair programming a while ago it is actually a very useful practice for ensuring that we don’t get stuck.

We’re much less likely to go off down a rabbit hole trying to solve some interesting but unrelated problem if we have to try and convince someone else to come along on that journey.

On most teams that I’ve worked on at least a reasonable percentage of the team is co-located so there’s almost certainly going to be someone sitting nearby who will be able to help.

If that isn’t enough, we tend to have a very visible story wall of what everyone’s working on right next to the work space and it become pretty obvious when something has been stuck in one of the columns for a long time.

Another team member is bound to point that out and if they don’t then the standup at the beginning of the day provides a good opportunity to see if anyone else on the team has a way around the problem you’re working on.

It also provides an opportunity to find out whether the problem you’re trying to solve is actually worth solving or not by talking to the product owner/one of the business analysts.

For the types of problems that I work on more often than not it isn’t vital to solve a lot of problems that we think we need to and the product owner would much rather we just parked it and work on something else that is valuable to them.

Jeff goes on to describe some other more general ways of getting unstuck but the above are some which might not be available to us with a less collaborative approach.

I recently worked on adding the meta data section for each of the different document types that it serves which involved showing 15-20 pieces of data for each document type.

There are around 4-5 document types and although the meta data for each document type is similar it’s not exactly the same!

When we got to the second document type it wasn’t obvious where the abstraction was so we went for the copy/paste approach to see if it would be any easier to see the commonality if we put the two templates side by side.

We saw some duplication in the way that we were building up each individual piece of meta data but couldn’t see any higher abstraction.

We eventually got through all the document types and hadn’t really found a clean solution to the problem.

I wanted to spend some time playing around the code to see if I could find one but Duncan pointed out that it was important to consider that refactoring in the bigger context of the application.

Even if we did find a really nice design it’s probably not going to give us any benefit since we’ve covered most of the document types and there will maybe be just one that we have to add the meta data section for.

The return on investment for finding a clean generic abstraction won’t be very high in this case.

In another part of our application we need to make it possible for the use to do faceted search but it hasn’t been decided what the final list of facets to search on will be.

It therefore needs to be very easy to make it possible to search on a new facet and include details about that facet in all search results.

We spent a couple of days about 5/6 weeks ago working out how to model that bit of code so that it would be really easy to add a new facet since we knew that there would be more coming in future.

When that time eventually came last week it took just 2 or 3 lines of code to get the new facet up and running.

In this case spending the time to find the generic abstraction had a good return on investment.

I sometimes find it difficult to know exactly which bits of code we should invest a lot of time in because there are always loads of places where improvements can be made.

Analysing whether there’s going to be a future return on investment from cleaning it up/finding the abstraction seems to be a useful thing to do.

Of course the return on investment I’m talking about here relates to the speed at which we can add future functionality.

I guess another return on investment could be reducing the time it takes to understand a piece of code if it’s likely to be read frequently.

As I’ve mentioned before, we’re making use of a MarkLogic database on the project I’m working on which means that we’re getting quite big XML data structures coming into our application whenever we execute a query.

The normal way that I’ve seen for dealing with external systems would be to create an anti corruption layer where we initialise objects in our system with the required data from the external system.

In this case we’ve decided that approach doesn’t seem to make as much sense because we don’t need to do that much with the data that we get back.

We effectively map straight into a read model where the only logic is some formatting for how the data will be displayed on the page.

The read model objects look a bit like this:

class Content(root : xml.Node) {
    def numberOfResults: Int = (root \ "@count").text.toInt
}

They are just lightweight wrapper objects and we make use of Scala’s XML support to retrieve the various bits of content onto the page.

The advantage of doing things this way is that it means we have less code to write than we would with the serialisation/deserialisation approach although it does mean that we’re strongly coupled to the data format that our storage mechanism uses.

However, since this is one bit of the architecture which is not going to change it seems to makes sense to accept the leakage of that layer.

So far the approach seems to be working out fine but it’ll be interesting to see how well it holds up if those lightweight wrappers do end up needing to have more logic in them.

It recently struck me while preparing some ThoughtWorks University sessions that there appear to be two modes that I spend my time switching between while coding:

• Action mode – we’re focused on getting things done, making things happen
• Reflective mode – we’re a bit more detached and looking at things from a higher level

I spent the majority of 2008 and 2009 in reflective mode on the systems I was working on which can be seen by scanning through a lot of the blog posts that I wrote during that time.

I’m sure would have been times when I was action mode but I was far more interested in how something was being built and whether that could be done more successfully.

In 2010/2011 I became much more interested in building stuff and spent more time thinking about that rather than how to refactor code or design it in a more functional way for example.

I’m now coming to the conclusion that both of these mentalities are useful at different times and I need to be more aware of when I’ve been spending too long in one or the other.

Examples of the two modes

When pairing programming these modes describe the role of the driver/navigator reasonably well.

While driving we’re head down focusing on building the required functionality whereas the navigator focuses more on quality of what’s being written, whether we can do that better, whether we’re going to cause ourselves big problems down the line and so on.

When we’re learning something new we’ll mostly be in action mode to start with – we want to see something working so that we get some feedback that we’re getting somewhere.

After a while when we’re a bit more comfortable with the language/tool/technique we’ll probably step back and reflect on how it’s going.

I think you can be in these two mindsets in non coding situations too.

For example I’ve recently become much more vocal in trying to get teams to use approaches that I’ve seen work before whereas previously I was happier to sit back and watch how things panned out and learn from that.

Are the modes this clear cut?

While writing this post I’ve started to wonder whether the way we use these two modes are actually this clearcut and whether I’m only aware of the ‘obvious’ switches between the two when in fact there are many more switches.

I’ll keep observing…

Something I’ve noticed when I play around with Ruby in my own time is that I nearly always end up with the situation where I’m passing hashes all over my code and to start with it’s not a big deal.

Unfortunately I eventually get to the stage where I’m effectively modelling an object inside a hash and it all gets very difficult to understand.

I’ve written a few times before about incrementally refactoring code so this seemed like a pretty good chance for me to try that out.

The code in the view looked something like this:

<% @tweets.each do |tweet| %>
  <%= tweet[:key] %>  <%= tweet[:value][:something_else] %>
<% end %>

@tweets was being populated directly from a call to CouchDB so to start with I needed to change it from being a collection of hashes to a collection of objects:

I changed the Sinatra calling code from:

get '/' do
  @tweets = get_the_couchdb_tweets_hash
end

to:

get '/' do
  tweets_hash = get_the_couchdb_tweets_hash
  @tweets = tweets_hash.map { |tweet| TweetViewModel.new(tweet) }
end

where TweetViewModel is defined like so:

class TweetViewModel
  attr_accessor :key, :value
 
  def initialize(tweet_hash)
    @key = tweet_hash[:key]
    @value = tweet_hash[:value]
  end
 
  def get(lookup)
    if lookup == :key
      key
    else
      value
    end
  end
 
  alias_method :[], :get
end

The next step was to get rid of the get method and rename those attr_accessor methods to something more intention revealing.

class TweetViewModel
  attr_accessor :url, :messages
 
  def initialize(tweet_hash)
    @url = tweet_hash[:key]
    @messages = tweet_hash[:value]
  end
end

<% @tweets.each do |tweet| %>
  <%= tweet.url %>  <%= tweet.messages[:something_else] %>
<% end %>

I originally didn’t realise how easy it would be to make the TweetViewModel pretend to temporarily be a Hash but it actually made it really easy for me to change the code and know that it was working the whole way.

For someone with more Ruby experience perhaps it wouldn’t be necessary to break out the refactoring like this because they could fairly confidently do it in one go.

While reading Dan North’s second post about software craftsmanship I was able to resonate quite a lot with a point he made in the ‘On value’ section:

I’m not going to mandate test-driving anything (which is a huge about-face from what I was saying a year ago), unless it will help. Copy-and-paste is fine too. (Before you go all shouty at me again, hold off until I blog about the benefits of copy-and-paste, as it appears in a couple of patterns I’m calling Spike and Stabilize and Ginger Cake. You might be surprised.)

I’ve been finding that quite frequently with some of the problems I’ve worked on recently that we haven’t known exactly how to solve it when we started and ended up hacking/spiking the code quite a bit at that stage until we figured out what we needed to do.

Dan replied with the following on twitter when I mentioned this to him:

@markhneedham kind of deliberately discovering the best way forward by actively reducing your ignorance? hmm, might catch on…

He wrote about this a few months ago in a post titled ‘Introducing Deliberate Discovery‘.

I nearly always feel pretty bad when I take this approach because I’m not TDDing code which easily be written using that style.

The reason I don’t is that it slows me down when I’m in discovery mode.

I could TDD what I think the code needs to look like and end up with nice looking code which doesn’t actually solve the problem but that seems to miss the point.

Another approach could be to drive from a higher level by using cucumber or a similar tool but that always ends up being quite fiddly from my experience and again it slows down the discovery that I want to make.

We had a recent example of this while trying to work out how to display the next part of a page we were working on.

Our initial approach was to make an AJAX call to the server and get back a JSON representation of our resource which we could render on the page.

While implementing this approach we realised that there were already a some post backs being made to the server from various other parts of the page which resulted in the page being refreshed.

We initially thought that this would mean that we could use the data being passed to the view on those post backs to do what we wanted.

We kept a copy of our original code and then started trying to implement what we thought would be a simpler solution.

Unfortunately after playing around with that approach for a few hours we realised that it wasn’t going to work because loading the data that way led to another part of the page getting completely screwed up.

We therefore ended up back at the first approach again.

One side effect of taking that second approach was that eventually the logic got too complicated for us to verify that what we had written was correct just by eye balling it.

We therefore slowed down a bit and TDD’d the code so that we could document our understanding of what was supposed to happen.

Uncle Bob wrote a reply to Dan a couple of days ago in which he ends with the following:

So when you see someone wearing a green wrist-band that says “Clean Code” or “Test First” or “Test Obsessed”
…

It’s a promise made to one’s self: “I will do a good job. I will not rush. I will write tests. I will go fast by going well. I will not write crap. And I will practice, practice practice so that I can be a professional.”

The code I’m writing during the time when I’m discovering what I need to do certainly isn’t clean or test first but it is part of an attempt to allow me to figure out what to do in order to go faster.

I’m more used to taking the approach I’ve described when explicitly working on ‘spikes’ or ‘experiments‘ but in a way what we’ve done is perhaps closer to concurrent set based engineering.

I think the skill I still quite haven’t got right is realising when I’ve taken the spiking/hacking too far and would benefit from going and TDDing a bit of code but I’m sure I’ll figure it out with practice.

I’ve been flicking through Growing Object Oriented Software Guided By Tests again and in Chapter 6 on Object Oriented Style I came across the part of the chapter which talks about writing context independent code which reminded me of some code I’ve worked on recently.

The authors suggest the following:

A system is easier to change if its objects are context-independent; that is, if each object has no built-in knowledge about the system in which it executes

I was writing a bit of code in our ApplicationHelper which would only be used in a certain context within one of the views.

The view code was roughly like this:

<% unless current_user.blank? %>
   <% if show_something_for(current_user) %>
      <!-- some html -->
   <% else %>
	 <!-- some other html -->
   <% end %>
<% end %>

with the ‘show_something_for’ method defined like so:

module ApplicationHelper
   def show_something_for(user)
      user.has_foo? and user.has_bar?
   end
end

Inside the ‘show_something_for’ method we’re working off the assumption that user will not be nil based on that fact that it’s being used inside a context where we’ve already checked that we do in fact have a user.

It’s not identical to the situation the authors are describing but there is an implicit assumption in this method which would mean that we couldn’t necessarily just go and use it anywhere else in the code base and assume that it’d work.

Having said that, the code is slightly simpler than if we had to assume that ‘user’ might be nil.

The situation in which we don’t have ‘current_user’ happens when no user has logged in so the method warden mixes into our ApplicationController returns nil.

I think it would be possible to make use of the null object pattern and store a guest user in the session but there are a fair few places in the code base that rely on the current implementation at the moment.

I just came across a really cool blog post written a couple of months ago by Evan Light where he proposes that we ‘write the first one ugly‘:

To overcome paralysis, for small chunks of code, it is often better to just write whatever comes to mind – no matter how awful it may seem at the time. Give yourself permission to let the first version suck.

I think this is a really good piece of advice and it seems along the same lines as a suggestion from Uncle Bob in Clean Code:

When I write functions they come out long and complicated…then I massage and refine that code, splitting out functions, changing names and eliminating duplication…all the whole keeping the tests passing.

I find myself following a similar approach to Evan these days whereas previously I’d probably have spent a lot of time thinking through the problem in my head trying to come up with a design I was happy with.

I agree with Evan that it’s frequently easier to see a clean way to solve a problem if you actually have some sort of code in front of you even if it is a terrible solution.

If I get stuck I tend to copy and paste other code, break encapsulation of objects, write long methods and so on. Then after I have something actually working I’ll go back and clean it up.

Sometimes I don’t see a way to write a piece of code more cleanly so I’ll leave it alone until the next time we’re working in that area of the code base, an approach that Joshua Kerievsky expands upon in a post he wrote a few months ago titled ‘sufficient design‘.

I’ll leave the final word to Simon Harris who made the following observation on twitter which I think is pretty accurate:

Ive said it before the difference between great developers and hacks is: the former clean up after themselves.

One of the earliest rules of thumb that I was taught by my colleagues is the idea that we should try and avoid mutating/changing values passed into a function as a parameter.

The underlying reason as I understand it is that if you’re just skimming through the code you wouldn’t necessarily expect the values of incoming parameters to be different depending where in the function they’re used.

I think the most dangerous example of this is when we completely change the value of a parameter, like so:

public class SomeClass
{
	public BigDecimal doSomeCalculationsOn(BigDecimal value) {   
		value = value.divide(new BigDecimal("3.2"));
		// some other calculation  on value...
		// and we keep on re-assigning value until we return the value
		return value;  
	}
}

In this case the function is really small so maybe it doesn’t make that much difference readability wise but I still think it would be better if we didn’t reassign the result of the calculation to ‘value’ but instead used a new variable name.

It wouldn’t require a very big change to do that:

public class SomeClass
{
	public BigDecimal doSomeCalculationsOn(BigDecimal value) {   
		BigDecimal newValue = value.divide(new BigDecimal("3.2"));
		// some other calculation  on newValue...
		// and we keep on re-assigning newValue until we return the newValue
		return newValue;  
	}
}

Unless the function in question is the identity function I find it very weird when I read code which seems to return the same value that’s been passed into the function.

The other way that function parameters get changed is when we mutate the values directly. The collecting parameter pattern is a good example of this.

That seems to be a more common pattern and since the function names normally reveal intent better it’s normally less confusing.

It does become more problematic if we’re mutating an object in loads of places based on conditional statements because we can lose track of how many times it’s been changed.

Interestingly some of the code for ActionPack makes use of both of these approaches in the same function!

form_options_helper.rb

564
565
566
567
568
569
570
571

      def to_collection_select_tag(collection, value_method, text_method, options, html_options)
        html_options = html_options.stringify_keys
        add_default_name_and_id(html_options)
        ...
        content_tag(
          "select", add_options(options_from_collection_for_select(collection, value_method, text_method, :selected => selected_value, :disabled => disabled_value), options, value), html_options
        )
      end

form_helper.rb

        def add_default_name_and_id(options)
          if options.has_key?("index")
            options["name"] ||= tag_name_with_index(options["index"])
          # and so on
          end
        end

I’m not sure how exactly I’d change that function so that it didn’t mutate ‘html_options’ but I’m thinking perhaps something like this:

	def create_html_options_with_default_name_and_id(html_options)
          options = html_options.stringify_keys
          if options.has_key?("index")
            options["name"] ||= tag_name_with_index(options["index"])
          # and so on
	end

And we could then change the other method to call it like so:

      def to_collection_select_tag(collection, value_method, text_method, options, html_options)
	   html_options_with_defaults = create_html_options_with_default_name_and_id(html_options)
        ...
        content_tag(
          "select", add_options(options_from_collection_for_select(collection, value_method, text_method, :selected => selected_value, :disabled => disabled_value), options, value), html_options_with_defaults
        )
      end

I guess you could argue that the new function is doing more than one thing but I don’t think it’s too bad.

Looking back on these code examples after writing about them I’m not as confident that mutating parameters is as confusing as I originally thought…!

One of the things that I’ve noticed as we’ve started writing more client side code is that I’m much more likely to look for a library which solves a problem than I would be with server side code.

A requirement that we’ve had on at least the last 3 or 4 projects I’ve worked on is to do client side validation on the values entered into a form by the user.

The jQuery.validate plugin is quite a good fit for this problem and as long as the validation is just on a field by field basis then it works fine.

On the last project I worked on, however, we had validation rules which had field interdependencies and suddenly we ended up writing a lot of custom code to handle that on top of what jQuery.validate already did.

Eventually we got to the stage where the code had become a complete mess and we decided to rewrite the validation code server side and only fire the validation when the user submitted the form.

In this situation that was an acceptable trade off to make but in another we may have needed to write our own Javascript code to handle the various validation rules.

In that case we’d probably want to write our own code to handle the inter field dependencies but still use jQuery.validate to handle individual field validation.

While thinking about this I was reminded of a post written by Michael Feathers back in 2003 where he discusses ‘stunting a framework’:

[...]let’s think about great frameworks… erm.. there aren’t many are there? In fact, even the good ones are a pain in the ass, aren’t they? There was that time when we downloaded framework X and it took quite a bit of time to learn how to use it, and that other time when we thought it would be useful if only it did that, but we spent the next week trying to force it and…”

Framework use is hard, yet we keep trying. Why do we do it? Mainly because we want to [...] leverage the work of others. If we use someone’s else framework, we may save some time. Moreover, we’ve benefited from someone’s “crystalized thought,” thought in the form of working code. The code shows a proven way of doing things and if it’s well-designed it can accommodate our thoughts and we can roll them back to the community.

Although the majority of the article is talking about frameworks from the angle of avoiding the temptation to create frameworks, I think it’s interesting to consider whether we always need to use a framework.

One other area where I’ve noticed we instinctively turn to a framework is when we have to interact with a database. The instinct is to straight away use Hibernate/NHibernate/ActiveRecord when frequently the initial use case doesn’t really require their use.

However, if we don’t make that decision up front then we need to be quite vigilant about observing the point at which we’re actually just reinventing the wheel rather than making a pragmatic decision not to use an ORM tool.

There are certainly other considerations to make when deciding whether to use a library or not such as our familiarity with it and its reputation/reliability in the community but it is still a decision point and one that I’ve frequently not recognised as being one and just gone straight for the library option.