Mark Needham

Thoughts on Software Development

Archive for the ‘Coding’ Category

Coding: Write the first one ugly

with 3 comments

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.

Written by Mark Needham

October 3rd, 2010 at 5:03 am

Posted in Coding

Tagged with

Coding: Mutating parameters

with 5 comments

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…!

Written by Mark Needham

August 26th, 2010 at 7:47 am

Posted in Coding

Tagged with

Coding: Using a library/rolling your own

with one comment

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.

Written by Mark Needham

August 10th, 2010 at 5:25 pm

Posted in Coding

Tagged with

Coding: Tools/Techniques influence the way we work

with 2 comments

Dave Astels mentions in his BDD paper that the way we use language influences the way that we write code, quoting the Sapir-Whorf hypothesis

“there is a systematic relationship between the grammatical categories of the language a
person speaks and how that person both understands the world and behaves in it.”

In a similar way, something which I didn’t fully appreciate until the last project I worked on is how much the tools and techniques that you use can influence the way that you work.

Distributed Source Control

Christian persuaded the client to allow us to use Mercurial for the project and it was interesting to see how we almost instinctively moved to a style of development which involved checking in much more frequently than we would have had we used Subversion.

There were still times when we’d end up working on something locally for too long but it seemed to become much more visible when this was happening and the endless ribbing that a pair got when they hadn’t checked in for a while ensured that it didn’t last for long.

I’m sure there are ways that we could have used Mercurial even more effectively than we did and my current thinking is that by default we’d want to use a distributed source control tool over any other.

Incremental refactoring

Incremental refactoring is a concept that Dave Cameron introduced me to about a year ago and it’s been talked about recently by Kent Beck and Joshua Kerievsky.

The underlying idea is that we know we want to drive our code in a certain direction but we want to do so in a way that doesn’t leave our code base in a broken state while we’re working towards that.

The techniques these two describe help to remove the fear and paralysis that we can often feel when we want to change a significant part of the code but know that we don’t have the time to do that all at once.

Not copying and pasting tests

I’ve previously mentioned a post Ian Cartwright wrote a while ago where he suggested that we should treat test code the same way that we treat production code, along the way pointing out that this meant copy/pasting tests was not the way to go.

I gave the non copy/paste approach a try last year and an interesting thing that I noticed is that when you have to type out the same types of things repeatedly you become much more aware of the duplication that you’re creating and since it’s costing time doing so you quickly look for ways to improve the situation.

Once we’ve got our tests cut down to the stage where removing any more duplication would also remove the intent of the test it doesn’t seem too bad to copy the outline of tests and then change the details.

We probably can move slightly faster by using copy/paste at this stage rather than writing everything out but the amount of time saved as a total of all development effort is minimal.

I think avoiding copy/paste puts us in a much more resourceful mindset and allows us to see improvements that we otherwise wouldn’t see and this it the real benefit of this technique.

Alan Skorkin has a cool post from a few months ago where he talks about the benefit of thinking about the code we’re writing rather than just going into auto pilot which covers similar ground.

Living prototypes

This is an idea that we used on my last project whereby we had a prototype of the website being developed alongside the real version.

This approach was necessary for this particular client but I really like the idea of having the real UI being developed alongside the code as it meant that whenever we showcased what we’d done to the client it was actually exactly what the final product would look like.

On previous projects we’ve often driven out the back end code and then styled it later which leads to a lot of questions about the UI when showcasing which may not actually be relevant when it’s properly skinned.

It also helped avoid the redundancy of showcasing something twice – once early on with just the functionality and then later on after the styling has been applied.

Written by Mark Needham

August 7th, 2010 at 1:14 pm

Posted in Coding

Tagged with

Technical Debt around release time

with 5 comments

One of the requirements that the ThoughtWorks University grads have been given on the internal project they’re working on is to ensure that they leave the code base in a good state so that the next batch can potentially continue from where they left off.

The application will be deployed on Thursday and this means that a lot of the time this week will be spent refactoring certain areas of the code base rather than only adding new functionality.

When this was suggested Duda pointed out that it’s often the case that we might accept a certain amount of technical debt in order to get the application out there.

While he is right and this is quite an unusual situation, we did see a similar situation on the last project I worked on.

On that project there was quite a tight delivery deadline for the first release so we knowingly incurred some technical debt in order to make sure that we met that date.

I’ve written previously about some of the technical debt that we incurred in that first release and while I think most of the time we made the right call I think there were still some occasions when we thought we were taking on deliberate prudent debt but were actually taking on deliberate imprudent debt.

Luckily it didn’t really come back to bite us and in the second release we had a much more relaxed pace and were therefore able to go through the code base and refactor certain parts of it to make it more maintainable.

kitchen.jpg

J.B. Rainsberger has a really cool analogy about refactoring where he talks about cleaning the kitchen and cleaning the garage.

Cleaning the kitchen is what we endeavour to do all the time such that we’ll write a bit of code and then clean up after ourselves. Sometimes we don’t clean up enough and we end up with a bit of a mess which takes much longer to clean up – i.e. we need to clean the garage.

garage.jpg

I think we sometimes drift towards thinking that we don’t need to clean the kitchen so often and end up cleaning the garage too often as a result.

This is something that Uncle Bob covered in a post he wrote around a year ago where where he points out that we’re more likely to take on technical debt when we didn’t need to rather than the other way around.

Finding the right balance between cleaning the kitchen and cleaning the garage seems to be something that comes from the experience of taking either approach too far.

Written by Mark Needham

July 25th, 2010 at 2:21 pm

Posted in Coding

Tagged with

Mikado-ish method for debugging

with 4 comments

I’ve written previously about the Mikado method and how I’ve made use of it for identifying ways in which I could refactor code but I think this approach is more generally applicable for any kind of code investigation.

Our application has a lot of calculations in it and we’ve been trying to refactor the code which wires all the calculators up to make use of a DSL which reveals the intention of the code more as well as making it easier to test.

Unfortunately after changing the code to use this approach one of the calculations was about by about £15.00 in one of our acceptance tests.

We didn’t have the best logging around all the calculators so it wasn’t immediately obvious where to start looking for the problem.

I decided to sketch out the ways the calculators interacted with each other and then follow the deepest path – which was Calculator D – Calculator G – Calculator H – to see if there were any differences in the values being calculated when running the old and new versions of the code.

mikado.jpg

Interestingly there was no problem in that bit of the code which then allowed me to rule out that whole part of the tree and then start looking at the other calculators to try and find the problem.

I’d previously been trying to work out what was going on just by reading through the code but found it incredibly difficult to remember where I’d already investigated so drawing the diagram really helped with that too.

Written by Mark Needham

July 4th, 2010 at 1:20 am

Posted in Coding

Tagged with

Coding: Having the design influenced by the ORM

without comments

I wrote a few weeks ago about incremental refactoring using a static factory method where we ended up with the following code:

public class LookUpKey
{
	private readonly string param1;
	private readonly string param2;
	private readonly string param3;
 
	public LookUpKey(string param1, string param2, string param3)
	{
		this.param1 = param1;
		this.param2 = param2;
		this.param3 = param3;
	}
 
	public static LookUpKey CreateFrom(UserData userData)
	{
		var param1 = GetParam1From(userData);
		var param2 = GetParam2From(userData);
		var param3 = GetParam3From(userData);
 
		return new LookUpKey(param1, param2, param3);
	}
 
	public string Param1Key
	{
		{ get { return param1; } }
	}
 
	...
}

The next step after this refactoring that we wanted to drive was to push the logic out of the static factory method so that it could just be done when those properties were evaluated.

We wanted to drive the code into something similar to this:

public class LookUpKey
{
	private readonly UserData userData;
 
	public LookUpKey(UserData)
	{
		this.userData = userData;
	}
 
	public static LookUpKey CreateFrom(UserData userData)
	{
		return new LookUpKey(userData);
	}
 
	public string Param1Key
	{
		{ get { return GetParam1From(userData); } }
	}
 
	...
}

Unfortunately we also hydrate the ‘LookUpKey’ from the database when we’re loading ‘LookUps’ into memory so that we can query them in memory.

public class LookupRepository
{
	public Lookup Find(LookupKey lookupKey)
	{
	     var query = currentSession.Linq<LookupRecord>();
 
		// this result would be cached and then queried but for the sake of the example I don't show that
 
	     var lookupRecord = query.Where(l => l.LookupKey.Equals(lookupKey)).FirstOrDefault();
 
		if(lookup == null) throw Exception(...);
 
		return lookupRecord.Lookup;
	}
}

We are therefore mapping directly to those fields to hydrate the object. The Fluent NHibernate mapping code looks like this:

public class LookupRecordMapping : ClassMap<LookupRecord>
{
    public LookupRecordMapping()
    {
        ...
 
        Component(lookupRecord => lookupRecord.LookupKey, lookupKey =>
        {
            lookupKey.Map(x => x.Param1).Access.CamelCaseField();
            lookupKey.Map(x => x.Param2).Access.CamelCaseField();
            lookupKey.Map(x => x.Param3).Access.CamelCaseField();
        });
    }
}

The database table for that is as follows:

Param1 | Param2 | Param3 | LookupValue1 | LookupValue2

Looking at the code like this makes it more obvious that ‘Param1′, ‘Param2′ and ‘Param3′ together represent a concept and that ‘UserData’ probably doesn’t belong inside that object in the first place.

Rafael Peixoto de Azevedo wondered in the comments on the original post whether ‘Param1′, ‘Param2′ and ‘Param3′ represent a relevant concept for ‘UserData’ but interestingly in this case the business guys didn’t have a name for this concept so we decided that it was some sort of lookup.

I found it interesting that looking at this code from a different part of the system made it clearer that the refactoring I wanted to make didn’t actually make sense.

Written by Mark Needham

July 2nd, 2010 at 4:56 pm

Posted in Coding

Tagged with

Coding: Controlled Technical Debt

with 2 comments

A couple of months ago I wrote about an approach to stories that Christian has been encouraging on our project whereby we slim stories down to allow us to deliver the core functionality of the application as quickly as possible.

In our case we had a requirement to setup a range of different parameters used to lookup reference data used in the different calculations that we have in our application.

At the time we created an interface that the rest of the application would interact with so that we could easily substitute the real version in when we needed to:


technicaldebt.jpg

We released the first version of the application about a month ago and finally last week implemented the story where the data would move from being in memory to being in a database table.

One of the requirements which we had delayed by only having these parameters in memory was the ability to easily modify them.

Any changes that needed to be made required an update of the code and then redeployment whereas with the database approach we would only have needed to deploy a delta script.

In the event there has only been one occasion so far where those parameters needed to be updated so it hasn’t proved to be a problem.

Discussing this with a colleague on Friday he pointed out that what we’d done originally was to accept technical debt in our solution knowing that at some stage in the future we would need to address that.

The interesting thing about this case was that we knew exactly when we were going to repay that debt whereas it’s often the case that we create technical debt in a code base and vaguely know that at some stage we’ll address it.

As Martin Fowler points out in his entry on technical debt:

Just as a business incurs some debt to take advantage of a market opportunity developers may incur technical debt to hit an important deadline.

We probably saved at least a day’s worth of effort by delaying this decision and were able to work on functionality that the business needed by the deadline instead. We then paid back that debt last week when we had more slack in the system.

The benefits of getting something into the market place quickly are much greater than I previously imagined and I think we can look at our assumptions of how a solution ‘needs’ to be designed much more closely to see if we can make these trade offs more frequently.

Written by Mark Needham

June 20th, 2010 at 10:37 pm

Posted in Coding

Tagged with

Incremental Refactoring: Create factory method

with 7 comments

Dermot and I spent a bit of time today refactoring some code where the logic had ended up in the wrong place.

The code originally looked a bit like this:

public class LookupService
{
	public LookUp Find(UserData userData)
	{
		var param1 = GetParam1From(userData);
		var param2 = GetParam2From(userData);
		var param3 = GetParam3From(userData);
 
		var lookupKey = new LookUpKey(param1, param2, param3);
		return lookupRepository.Find(lookupKey);
	}
}
public class LookUpKey
{
	private readonly string param1;
	private readonly string param2;
	private readonly string param3;
 
	public LookUpKey(string param1, string param2, string param3)
	{
		this.param1 = param1;
		this.param2 = param2;
		this.param3 = param3;
	}
}

We wanted to push the logic used to create the params in ‘LookUpKey’ into the ‘LookUpKey’ since it seemed that was a more natural place for that behaviour to live.

Unfortunately there were also a few other places that were use LookUpKey’s constructor so we couldn’t just go and change that to take in a ‘UserData’ otherwise we’d break other places in the code base.

My initial thought was that we could overload the constructor and temporarily have two constructors until we got rid of the original.

Dermot pointed out a better approach which was to add a static factory method to ‘LookUpKey’ and initially push the logic into that method.

public class LookUpKey
{
	...
 
	public static LookUpKey CreateFrom(UserData userData)
	{
		var param1 = GetParam1From(userData);
		var param2 = GetParam2From(userData);
		var param3 = GetParam3From(userData);
 
		return new LookUpKey(param1, param2, param3);
	}
}

The service is now much simplified:

public class LookupService
{
	public LookUp Find(UserData userData)
	{
		var lookupKey = LookUpKey.CreateFrom(userData);
		return lookupRepository.Find(lookupKey);
	}
}

We can also move tests that were originally indirectly testing the key creation by checking what was passed to the repository to be directly against the ‘LookUpKey’.

Eventually we want to make LookUpKey’s constructor private but for now we’ve been able to make our change without breaking the code base elsewhere.

Written by Mark Needham

June 17th, 2010 at 12:43 am

The Refactoring Dilemma

with 8 comments

On several of the projects that I’ve worked on over the last couple of years we’ve seen the following situation evolve:

  • The team starts coding the application.
  • At some stage there is a breakthrough in understanding and a chance to really improve the code.
  • However the deadline is tight and we wouldn’t see a return within the time left if we refactored the code now
  • The team keeps on going with the old approach
  • The project ends up going on for longer than the original deadline
  • It’s now much more work to move towards the new solution

In the situations I describe the refactorings could have been done incrementally but doing that would take longer than continuing with the original approach and also leave the code in an inconsistent state.

I think the reason this situation evolves consistently in this manner is because although we talk about writing maintainable code, delivery is often considered more important. Pushing out a delivery date in order to refactor code so that it will be easier to work with in the future isn’t something that I’ve seen happen.

Pushing a delivery date out is a cost that we can see straight away.

On the other hand it’s quite difficult to estimate how much of a gain you’ll get by refactoring to a more maintainable/easier to test solution and that gain will not be immediate.

We therefore end up in the situation where we tend to make major refactorings only if we’re going to see a benefit from doing that refactoring before the project ends.

In one sense that seems reasonable because we’re trying to ensure that we’re adding as much value as possible while the client is paying for our time.

On the other hand we’re making life harder for future maintainers of the code base which may in fact be us!

I’d be keen to hear how others handle these types of situations because it feels like this trade off is quite a common one and the way we’ve dealt with it doesn’t seem optimal.

Written by Mark Needham

June 13th, 2010 at 1:37 pm

Posted in Coding

Tagged with