Mark Needham

I wrote a couple of weeks ago about using the ‘call’ and ‘apply’ functions in Javascript when passing functions around and while working on our IE6 specific code I realised that I’d got them mixed up.

We were writing some code to override one of our functions so that we could call the original function and then do something else after that.

The code was roughly like this:

Foo = {
    bar : function(duck) {
      console.log("bar " + duck.quack());  
    }
};

The code that I originally wrote to capture the original function, call it and then do the additional behaviour was like this:

(function() {
  var originalBar = Foo.bar;
 
   Foo.bar = function(duck) {
        originalBar.call(this, arguments);
        console.log("new bar");
   };
})();

When we call the function:

Foo.bar({ quack : function() { return "quacking" } });

We get the following error:

TypeError: duck.quack is not a function

‘arguments’ is a local variable in any Javascript function which contains all the arguments passed to the function stored in an array type structure.

However, I had forgotten that when using the ‘call’ function we need to pass the full list of parameters individually rather than as an array so in this case we would need to pass ‘duck’ in specifically:

(function() {
  var originalBar = Foo.bar;
 
   Foo.bar = function(duck) {
        originalBar.call(this, duck);
        console.log("new bar");
   };
})();

Now when we run the function we get the expected behaviour:

Foo.bar({ quack : function() { return "quacking" } });

bar quacking
new bar

This is where apply comes in handy because apply allows us to pass in ‘arguments’ as the second parameter and it will send all the arguments of the function that we’re inside to the function that we’re calling which is exactly what we want in this case.

Using ‘apply’ we would end up with the following code:

(function() {
  var originalBar = Foo.bar;
 
   Foo.bar = function(duck) {
        originalBar.apply(this, arguments);
        console.log("new bar");
   };
})();

In this case the function only takes in one argument so there’s not much noticeable improvement in the code but when a function takes multiple arguments then using ‘apply’ is certainly a cleaner approach.

One thing we’ve found on my current project is that despite our best efforts we’ve still ended up with some javascript code which we only want to run if the user is using Internet Explorer 6 and the question then becomes how to write that code so that it doesn’t end up being spread all over the application.

jQuery has some functions which allow you to work out which browser’s being used but I’ve noticed that when we use those you tend to end up with if statements dotted all around the code which isn’t so good.

An approach which I was shown recently involves using CSS conditionals to identify when we’re using Internet Explorer instead.

We can then include an IE6 specific javascript file like so:

<!--[if lt IE 7]>
	<script type="text/javascript" src="/path/to/ie6.js") %>"></script>
<![endif]-->

Since we’re building an ASP.NET MVC application we include this bit of code in our master page so that it gets picked up by all the web pages.

We’ve either needed to override existing functions or call the existing function but then do some extra work afterwards as well.

In order to do this we have to make sure that the IE6 specific file is included after our other javascript files since the interpreter will use the last definition of a function that it finds.

Given an existing function defined like so:

Foo = {
    Bar : function() {
            console.log("original bar call");
    }
};

If we want to override this function to do something else we could include the following code in our IE6 specific file:

Foo.bar = function() {
    console.log("overriding bar call");
}

When we call ‘Foo.bar()’ we’d only see the second ‘console.log’ statement.

It becomes a bit more interesting if we want to call the original function and then do some other functionality.

We can make use of the proxy pattern to allow us to do this cleanly.

Foo = {
    bar : function() {
            console.log("original bar call");
    }
};
 
(function() {
    var originalBar = Foo.bar;
    Foo.bar = function() {
        originalBar.apply(this, arguments);
        console.log("overriding bar call");
    };
})();

If we call ‘Foo.bar()’ in IE6 we’d now see both of those ‘console.log’ statements.

The reason that we wrap the reassignment in a function is so that we can hide the ‘originalBar’ function from the rest of our code. We save ‘Foo.bar’ in a closure and then override it and delegate calls to the original before logging the extra message.

I quite like this approach although I’m not sure if it’s the most intention revealing code because it’s not necessarily obvious that the function is being rewritten unless you happen to know about the IE6 only file.

Is there a better way to do this than the approach I’ve described?

I came across a blog post by Rachel Davies where she wonders whether the Shu-Ha-Ri approach to learning/teaching is actually harmful and I found Rachel’s thoughts around the teaching of principles and practices quite interesting.

Quoting Jeff Sutherland:

Only when you have mastered the basic practices are you allowed to improvise. And the last and most important – Before you have gained discipline, centering, and flexibility, you are a hazard to yourself and others.”

I’m uncomfortable with approaches that force students to follow agile practices without questioning. These approaches seem to violate the first value of the Agile Manifesto “Individuals and interactions over processes and tools.”

What I find interesting about this is that when I first started learning about agile I would read the principles and they would all make sense but I didn’t know what to do with that knowledge.

It was only after I’d used the practices frequently and in most cases taking the practices too far that I reached the stage where I could start asking questions that actually had any relevance.

For example Test Driven Development is one practice that I learnt and one of the principles which encourages this approach to coding is the need to get quick feedback on our work.

It would be quite easy to question whether we should test drive everything and you could quite correctly have the opinion that we shouldn’t use this approach for absolutely everything.

However, without spending some time following this practice that opinion wouldn’t be particularly useful because you would lack the experience that tells you which type of code we should or should not look to test first.

Having decided that TDD was the only approach to writing code I eventually got into situations where it didn’t seem to make sense to stick so rigidly to this practice and it was only then that the value of the underlying principle became clear to me.

I don’t think I was ever forced not to ask questions but I was just so concentrated on learning how to do the practices properly that I focused on that.

I often find that it’s useful to use practices too much until you cause yourself pain.

Martin Fowler has a nice graph which shows how people adopt meta programming when coding Ruby and I think this links quite closely to my experiences when learning a new practice.

For me at least this seems to be a necessary learning step before I eventually step back and think about the principle that originally led me to following that practice. I can then start using the practice again but in a more effective or intelligent way.

What I’ve described above is often the way that I learn new things but I’m sure that others’ experiences will vary so it’d be interesting to hear about other approaches too!

On a few projects that I’ve worked on one of the things that we’ve done is create a shared library of objects which can be used across several different projects and while at the time it seemed like a good idea, in hindsight I’m not sure if it’s an entirely successful strategy.

I’m quite a fan of not recreating effort which is generally the goal when trying to pull out common code and within one team this seems to be a good approach the majority of the time.

When it comes to sharing across teams then I think we need to consider the perceived benefits a bit more because it doesn’t come without costs.

These are some of the types of code that we’ve shared previously:

Domain objects

I think this is the most dangerous type of code to share because although we often do have the same domain concepts in different projects, it’s quite rare that they mean exactly the same thing.

In addition there is an implicit coupling created with our database since we pretty much now have to make sure that our database schema matches up with the current version of that domain object.

Either that or we do have a shared database for all the applications which use that shared domain object in which case we have an even stronger coupling between applications.

We’re assuming that the two application have exactly the same domain concept and from my experience quite often that isn’t the case – even if there is a concept with the same name it may be used in different ways or mean something completely different in different applications.

This is quite similar to the problem with having a universal domain model which Dan points out in his classic SOA article.

In general I don’t think it makes sense to share this type of code.

Test code

This one seems like it should fairly universally a good idea – after all we often import 3rd party testing libraries so it seems like just sharing some common testing code shouldn’t be much different.

One piece of code that we shared was the Selenium bootstrapping code and this approach worked reasonably well until we wanted to adjust the amount of time between each command because commands were being sent to the browser before elements had the chance to load.

Apart from the fact that the other users of the library didn’t want anything change with respect to how they used the code we had to go and make the change in another project, build that and then update the reference that we had to the library.

Certainly this process would have been made easier if we’d used something like Ivy but the amount of duplication of code that we were saving didn’t seem worth the hassle it caused so we ended up inlining the code.

Infrastructure code

General infrastructure code e.g. code to handle NHibernate transactions which is quite unlikely to change seems one candidate which can work quite well in a shared library and so far I haven’t seen many problems arise from doing this.

I think the key with these bits of reusable code is that we keep them quite small and ensure that they have only one responsibility which will be useful for all the applications.

We eventually ended up slimming down our shared library and the majority of the code that remains in there is solving specific infrastructure type problems which will be the same across any applications using the same technical stack.

Things to be careful about when sharing code

One reason that we may share code is so that if there is a change then it only needs to be done in one place.

We need to have a degree of confident that if we put code in a shared library that this is actually the case.

If it’s likely that different applications might need shared code to change in different ways then we might not want to make that bit of code shared otherwise we’ll just end up with application specific code in a shared library.

From what I’ve noticed it makes most sense to put code which is unlikely to change and is generic enough to be useful across several applications as is into shared libraries.

For any other code it might actually be beneficial to accept that there will be some duplication between applications in the same organisation and not try and pull out a common piece.

I came across a couple of quite interesting blog posts recently which described some approaches to interviewing which suggest a more empirical approach to interviewing whereby the interview is treated more like an audition for the person being interviewed.

I like this idea and it’s something that we do when recruiting developers in a pair programming interview.

The general idea is that we pair with the candidate as they go through a coding problem. It’s perhaps a little different from a normal pairing session in that the interviewee is more than likely driving for the majority of the session.

Sometimes we might also have another interviewer observing the pairing session and giving input where necessary.

While many people may not have specifically pair programmed before nearly everyone has worked with someone else at one computer on a problem so I haven’t ever found that a candidate finds it too much of a leap because they’re used to working alone.

One of the really cool things about a pair programming interview is that it’s much closer to what a real situation on a project would be like which I think helps you to gain a more accurate picture of the skill level and potential of the interviewee.

As an interviewer you get the chance to see how quickly the interviewee will pick up new ideas, to an extent how well they work with other people and first hand experience of their level of expertise when it comes to coding.

From my experience of other types of interviews it can be quite difficult to tell exactly how much knowledge someone has in a specific topic but in a pair programming you can just see for yourself so it works out quite well in that sense.

It’s certainly not fool proof and I quite like the way that Hash Rocket have taken this idea to the next level and get people to come there and pair program with them for a week before they get hired.

This seems like the next logical step and I guess if you have the ability to do this and candidates are prepared to give up their time then it’s a really useful approach.

As Knut Haugen points out, the interview process is as much an opportunity for the employee to work out if they actually want to work for the potential employer and I think having the opportunity to pair with some of the people who work there is a great chance to assess this.

Dave Nicolette suggests that the pairing part of the interview process is considered the only meaningful part of the interview process in the team he’s coaching and while I’m not I’d go that far I do think it’s a very valuable approach and one I’d recommend even if you don’t pair program all the time on your team.

I wrote previously about how I’ve been using effect sketches to identify responsibilities in objects so that I can pull them out into other objects and once I’ve done this I often find that I can’t see a small next step to take.

At this stage in the past I’ve often then stopped and left the refactoring until I have more time to complete it but this hasn’t really worked and a lot of the time I end up only seeing the code change in my mind and not in the actual code.

I came across this problem again last week in an object which had 8 dependencies when I came across it.

Having drawn the effect sketch I realised that 3 of those could be pulled out into a new object which could then be injected into the original object and help to encapsulate those other 3 dependencies.

The code that I wanted to change was something like this:

public class TheObject 
{
	private readonly DependencyA;
	private readonly DependencyB;
	private readonly DependencyB;
	...	
 
	public Foo FooCreation()
	{
		var dependencyAValue = dependencyA.GetSomething();
		var dependencyBValue = dependencyB.GetSomething();
		var dependencyCValue = dependencyC.GetSomething();
 
		return new Foo(dependencyAValue, dependencyBValue, dependencyCValue);
	}
 
	...
}

I wanted to pull the ‘FooCreation’ method out into another object and then change all the places that were calling ‘TheObject’FooCreation’ to just call this new object directly.

The first step was to create a ‘FooFactory’ and just have that delegated to internally:

public class FooFactory 
{
	private readonly DependencyA;
	private readonly DependencyB;
	private readonly DependencyB;
	...	
 
	public Foo Create()
	{
		var dependencyAValue = dependencyA.GetSomething();
		var dependencyBValue = dependencyB.GetSomething();
		var dependencyCValue = dependencyC.GetSomething();
 
		return new Foo(dependencyAValue, dependencyBValue, dependencyCValue);
	}
 
}

public class TheObject 
{
	...	
 
	public Foo FooCreation()
	{
		return new FooFactory(dependencyA, dependencyB, dependencyC).Create();
	}
 
	...
}

I ran out of time at this stage to finish off the refactoring but it was obvious where the refactoring was going so the next time I got the chance I injected the ‘FooFactory’ into ‘TheObject’:

public interface IFooFactory 
{
	Foo Create();
}

public class TheObject 
{
	private readonly IFooFactory;
	...	
	public TheObject(IFooFactory fooFactory) 
	{
		this.fooFactory = fooFactory;
	}
 
	public Foo FooCreation()
	{
		return fooFactory.Create();
	}
 
	...
}

To do this I had to go and change the tests on ‘TheObject’ and move some of them to go directly against ‘FooFactory’.

The third stage of the refactoring was to change all the places which called ‘TheObject.FooCreation()’ to just call ‘FooFactory.Create()’ directly.
Some of those places were also using other methods on ‘TheObject’ so those objects now have an extra dependency although I think at least the code is more intention revealing than it was previously.

I’m sure there are some other patterns for this type of small step refactoring but this is just one that I’ve noticed so far.

I’ve written previously about how useful I find effect sketches for helping me to understand how an object’s methods and fields fit together and while drawing one a couple of weeks ago I noticed that it’s actually quite useful for seeing which parts of the code will be the easiest to change.

I was fairly sure one of the object’s in our code base was doing too many things due to the fact that it had a lot of dependencies.

However, it wasn’t obvious to me from looking at the code which would be the easiest place to start in pulling out some of those responsibilities.

I therefore drew out an effect sketch which looked something like this:

From the diagram I could see more clearly that ‘MethodC’ is using 3 fields which are not used by any of the other methods in the object.

This therefore seemed like the perfect method to pull out since I could do so really easily and get rid of 3 of the object’s fields since noone else used them anyway.

This reminded me a lot of the Mikado method for addressing technical debt which I’ve read about but haven’t used yet.

As I understand it, the goal with the Mikado method is to locate areas of the code base that we can change easily because there are no dependencies on this piece of code.

When using effect sketches the goal is to try and use the sketch to work out how we can group functionality and I think the idea of making initial changes that have a low impact is a good one to follow.

I drew my initial effect sketch on paper but I noticed that drawing it up in graphviz actually makes it even more obvious which bits of functionality are related.

For example I hadn’t realised that ‘fieldB’ was used by so many methods until I typed this up.

It’s quite a neat tool and easy to pick up. This is my ‘dot’ file for the above sketch:

blog.dot

digraph effectgraph {
	size="8,8"; 
 
	"MethodA" -> "fieldA";
	"MethodA" -> "fieldB";
	"MethodA" -> "fieldC";
 
	"MethodB" -> "fieldB";
	"MethodB" -> "fieldE";
	"MethodB" -> "fieldF";
 
	"MethodC" -> "fieldG";
	"MethodC" -> "fieldH"
	"MethodC" -> "fieldD"
 
	"MethodD" -> "fieldB"
	"MethodD" -> "fieldI"
}

And to generate a png I ran the following command from the terminal:

dot -Tpng -blog.png blog.dot

I spent some time over the weekend playing with the bowling game kata in Javascript.

I thought I knew the language well enough to be able to do this kata quite easily so I was quite surprised at how much I struggled initially.

These are some of my observations from this exercise:

• I was using screw-unit as my unit testing framework – I originally tried to setup JSTestDriver but I was having problems getting that to work so in the interests of not shaving the yak I decided to go with something I already know how to use.

  I don’t think I quite get the idea of the ‘describe’ and ‘it’ blocks which I believe are inspired by Rspec.

  I like the idea of describing the behaviour of an object for the different contexts in which it’s used but I found myself putting all my examples/tests in the same describe block without realising!

  I went back and tried to find the different contexts but the only obvious distinction I noticed was that some tests seemed to be covering fairly basic bowling combinations while others were covering specific types of games:

  • normal games
    • should score a single throw
    • should score two throws which do not add up to a strike or spare
    • should score a spare
    • should score multiple spares
    • should score a strike
    • should score back to back strikes
    • should score a combination of strikes and spares
  • special combinations
    • should score a full house of strikes
    • should score a full house of spares
    • should score a gutter game
    • should score a dutch 200

  There’s no specific setup unique to these two contexts which I’ve often noticed is a tell tale sign when writing tests in C# that we need to split our tests out a bit so I’m not sure whether I’ve added much value by doing this refactoring.

  Following Esko Luontola’s terminology the tests that I’ve written follow example style test names rather than specification style test names.

  This means that in order to understand the scoring rules of bowling you would need to look at the implementation of the test rather than just read the name.

  I think this might be a key difference in the way we write tests in JUnit/NUnit and RSpec/screw-unit.

• At one stage I was making use of the Array ‘splice‘ function to get an array with an element removed and I had expected that I would be returned a new array with those elements removed.

  In actual fact that function mutates the original array so if we’re going to do anything using ‘splice’ then it seems like we need to get a copy of the original array by using ‘slice‘ otherwise we may end up with some quite unexpected behaviour later on in the program.

  The other thing I found strange is that ‘splice’ returns the elements that have been removed from the array rather than the newly mutated array.

  To get an array with the first element removed we’d do something like this:

  function removeFirstItemFrom(theArray) {
  	var copy = theArray.slice(0);
  	copy.splice(0, 1);
  	return copy;	
  }
   
  var anArray = [1,2,3,4,5];
  var anArrayWithFirstItemRemoved = removeFirstItemFrom(anArray);

  After my time playing around with functional approaches to programming it’s quite strange to see APIs which mutate values and don’t return the results that I’d expect them to. Interesting though.

• Since a lot of the test setup involved rolling gutter balls I had a lot of calls to ‘bowlingGame.roll(0)’ which was making the test quite convoluted and not adding much value.

  I wrote a function to extend ‘Number’ so that I could use a Ruby style ’10.times’ syntax:

  Number.prototype.times = function(f) {
  	for(var i=0; i < this; ++i) {
  		f();
  	}
   
  	return this;
  };

  When I tried to use this function like this:

  10.times(function() { bowlingGame.roll(0); });

  I kept getting the following error:

  SyntaxError: missing ; before statement

  I couldn’t work out what I was doing wrong but skim pointed out that in Javascript we need to wrap number literals in parentheses in order to call functions on them:

  (10).times(function() { bowlingGame.roll(0); });

  works much better!

  I wrote a couple of other general use functions and one thing which I’m not sure about is whether or not I should do validation on the input parameters or is it down to the user of the function to use it correctly?

  I’m more used to static languages where it would be more difficult to pass in an unexpected value so I’m not sure what the normal approach would be in a dynamic language.

• I took quite a lot of ideas from the way Brett Schuchert solved the problem in Ruby.

  In particular I really like the way that he’s broken down the problem into smaller and smaller functions which all do only one thing. It’s quite easy to end up writing really complicated functions which are difficult to understand so it was good to see that it is possible to keep it this simple.

  It would be quite interesting to see how this type of solution evolved.

• This wasn’t my most incremental bit of coding ever – I found that some of the examples I introduced e.g. scoring a strike often resulted in quite a lot of code needing to change to make the test pass.

  My current thinking is that for this problem we perhaps need to have some idea of the way that we want the code to evolve before we start writing our solution. A solution doesn’t just evolve in front of us when we add in the next test. Either that or I’m not taking steps which are small enough to allow that evolution to happen.

I recently came across an interesting bit of code which was going through a collection of strings and then only taking the first ‘x’ number of characters and discarding the rest.

The code looked roughly like this:

var words = new[] {"hello", "to", "the", "world"};
var newWords = new List<string>();
foreach (string word in words)  
{
    if (word.Length > 3)
    {
        newWords.Add(word.Substring(0, 3));
        continue;
    }
    newWords.Add(word);
}

For this initial collection of words we would expect ‘newWords’ to contain ["hel", "to", "the", "wor"]

In a way it’s quite annoying that the API for ‘Substring’ throws an exception if you try and get just the first 3 characters of a string which contains less than 3 characters. If it didn’t do that then we would have an easy ‘Select’ call on the collection.

Instead we have an annoying if statement which stops us from treating the collection as a whole – we do two different things depending on whether or not the string contains more than 3 characters.

In the spirit of the transformational mindset I tried to write some code using functional collection parameters which didn’t make use of an if statement.

Following this idea we pretty much have to split the collection into two resulting in this initial attempt:

var newWords = words
    .Where(w => w.Length > 3)
    .Select(w => w.Substring(0, 3))
    .Union(words.Where(w => w.Length <= 3).Select(w => w));

This resulted in a collection containing ["hel", "wor", "to", "the"] which is now in a different order to the original!

To keep the original order I figured that we needed to keep track of the original index position of the words, resulting in this massively overcomplicated version:

var wordsWithIndex = words.Select((w, index) => new { w, index });
 
var newWords = wordsWithIndex
               .Where(a => a.w.Length >= 3)
               .Select((a, index) => new {w = a.w.Substring(0, 3), a.index})
               .Union(wordsWithIndex.Where(a => a.w.Length < 3).Select(a => new { a.w, a.index }))
               .OrderBy(a => a.index);

We end up with a collection of anonymous types from which we can get the transformed words but it’s a far worse solution than any of the others because it takes way longer to understand what’s going on.

I couldn’t see a good way to make use of functional collection parameters to solve this problem but luckily at this stage Chris Owen came over and pointed out that we could just do this:

var newWords = words.Select(w => w.Length > 3 ? w.Substring(0, 3) : w);

I’d been trying to avoid doing what is effectively an if statement inside a ‘Select’ but I think in this case it makes a lot of sense and results in a simple and easy to read solution.

One of the things I find most confusing when reading about programming languages is the idea of covariance and contravariance and while I’ve previously read that covariance is not possible when using generics in C# I recently came across an example where I saw that this was true.

I came across this problem while looking at how to refactor some code which has been written in an imperative style:

public interface IFoo 
{
	string Bar { get; set; } 
}
 
public class Foo : IFoo 
{ 
	public string Bar { get; set; }
}

private IEnumerable<IFoo> GetMeFoos()
{
    var someStrings = new[] { "mike", "mark" };
 
    var someFoos = new List<IFoo>();
    foreach (var s in someStrings)
    {
        someFoos.Add(new Foo { Bar = s });
    }
    return someFoos;
}

I changed the code to read like so:

private IEnumerable<IFoo> GetMeFoos()
{
    var someStrings = new[] { "mike", "mark" };
    return someStrings.Select(s => new Foo { Bar = s });
}

Which fails with the following compilation error:

Error	1	Cannot implicitly convert type 'System.Collections.Generic.IEnumerable<Test.Foo>' to 'System.Collections.Generic.IEnumerable<Test.IFoo>'. An explicit conversion exists (are you missing a cast?)

I thought the compiler would infer that I actually wanted a collection of ‘IFoo’ given that I was returning from the method directly after the call to Select but it doesn’t.

As I understand it the reason that we can’t downcast an IEnumerable of ‘Foo’ to an IEnumberable of ‘IFoo’ is that we would run into problems if we worked of the assumption that our original collection only contained Foos in it later on in our program.

For example it would be possible to add any item which implemented the ‘IFoo’ interface into the collection even if it wasn’t a ‘Foo’:

// this code won't compile
 
List<Foo> foos = new List<Foo>();
// add some foos
 
List<IFoo> ifoos = foos;
 
foos.Add(new SomeOtherTypeThatImplementsIFoo());

It’s not possible to convert ‘SomeOtherTypeThatImplementsIFoo’ to ‘Foo’ so we would run ourself into problems.

Rick Byers has a post from a few years ago where he explains how this works in more detail and also points out that covariance of generics is actually supported by the CLR, just not by C#.

In the case I described we can get around the problem by casting ‘Foo’ to ‘IFoo’ inside the ‘Select’:

private IEnumerable<IFoo> GetMeFoos()
{
    var someStrings = new[] { "mike", "mark" };
    return someStrings.Select(s => (IFoo) new Foo { Bar = s });
}