Mark Needham

I've been playing around with a little script in Clojure to parse the ThoughtWorks Blogs RSS feed and then create a tweet for each of them which contains a link to the blog post and the person's Twitter ID if they have one.

It's not finished yet but I'm finding the way that we parse documents like this in Clojure quite intriguing.

The xml to parse looks roughly like this:

<rss version="2.0"> 
	<channel> 
 		...
		<item> 
			<title>Simon Brunning: Links for 2009-11-27 [del.icio.us]</title> 
			<link>http://feedproxy.google.com/~r/SmallValuesOfCool/~3/WDqeLyMA-RE/brunns</link> 
		</item>
		<item> 
			<title>Alex Hung: Extending iPhone battery life</title> 
			<link>http://alexhung.vox.com/library/post/extending-iphone-battery-life.html?_c=feed-atom-full</link> 
		</item>
		...
	</channel> 
</rss>

I've only included the parts of the document that I'm interested in getting.

Following the examples from Stuart Halloway's book one approach to do this is to make use of the 'clojure.xml.parse' and 'clojure.core.xml-seq' functions to create a sequence representing the tree structure of the feed.

I'm used to parsing XML with XPath but that doesn't make as much sense when we have a sequence of hash maps. Instead I'm using 'filter' and 'map' to try and achieve the same outcome.

I found that while I was trying to work out how to use these functions together I was often trying to solve the whole problem in one go instead of breaking it down into smaller more manageable pieces.

I also noticed that I was using 'filter' more often than I needed to instead of filtering the data to the point that everything I wanted to extract was in the remaining data set.

When I was playing with F# I got into the habit of trying to minimise the number of intermediate values I created but this seemed to be making life more difficult so I've allowed myself some intermediate values for the moment!

The goal is to poll the ThoughtWorks RSS feed and then update the planettw account with the latest blog posts. The current setup does that but doesn't include people's Twitter names in the tweet so I'm trying to sort that out.

This is the code I have so far:

(use '[clojure.xml :only (parse)])
(def feed (xml-seq (parse (java.io.File. "clojure-play/tw-blogs-rss.txt"))))
 
(def rss-entries (filter #(= :item (:tag %)) feed))
 
(defn- get-href [link]
  ((comp :href :attrs) link))
 
(defn- get-value [node]
  (first (:content node)))
 
(defn rss-link [entry]
  (get-value (first (filter #(= :link (:tag %))
                            (:content entry)))))
 
(defn rss-title [entry]
  (get-value (first (filter #(= :title (:tag %))
                            (:content entry)))))
 
(def rss-titles (map #(rss-title %) rss-entries))
(def rss-links (map #(rss-link %) rss-entries))
 
(defn- get-author [title]
  (second (first (re-seq #"([\w ]+):" title))))
 
(defn- get-title [title]
  (second (first (re-seq #"[a-zA-Z0-9 ]+:\s(.*)" title))))
 
(def authors (map #(get-author %) rss-titles))
(def titles (map #(get-title %) rss-titles))
 
(defn- get-display-name [twitter-names real-name]
  (let [twitter-name (twitter-names real-name)]
    (if twitter-name (str "@" twitter-name) real-name)))
 
(def twitter-names {"Mark Needham" "markhneedham"
                    "Alex Hung" "alexhung"
                    "Simon Brunning" "brunns"
                    "Ola Bini" "olabini"
                    "Patrick Kua" "patkua"
                    "Marc McNeill" "dancingmango"
                    "Dahlia Bock" "dlbock"
                    "Sumeet Moghe" "sumeet_moghe"
                    "Brian Guthrie" "bguthrie"
                    "Ian Robinson" "iansrobinson"
                    "Ian Cartwright" "cartwrightian"
                    "Duncan Cragg" "duncancragg"
                    "David Cameron" "davcamer"
                    "Steven List" "athought"
                    "Philip Calcado" "pcalcado"
                    "Perryn Fowler" "perrynfowler"
                    "Jason Yip" "jchyip"
                    "Christopher Read" "cread"
                    "Jim Webber" "jimwebber"
                    "John Hume" "duelin_markers"
                    })
 
(defn- create-blog-post [title link author]
  {:tweet (str title " by " (get-display-name twitter-names author) " " link)})
 
(defn create-blog-posts [titles links authors]
  (map #(create-blog-post %1 %2 %3) titles links authors))

To use that you'd need to do this:

(create-blog-posts titles rss-links authors)

Which returns a sequence of hash maps with key 'tweet' and a value of the tweet to display on Twitter:

({:tweet "Links for 2009-11-27 [del.icio.us] by @brunns http://feedproxy.google.com/~r/SmallValuesOfCool/~3/WDqeLyMA-RE/brunns"} 
{:tweet "Extending iPhone battery life by @alexhung http://alexhung.vox.com/library/post/extending-iphone-battery-life.html?_c=feed-atom-full"} 
{:tweet "Threshold Anxiety by Adrian Wible http://thoughtadrian.blogspot.com/2009/11/threshold-anxiety.html"})

The next step is to get this hooked up to the Twitter API.

There are still some things I'm unsure of when it comes to writing applications in Clojure:

• I'm not sure what to do with 'twitter-names'. It's pretty much a global data store so I can't decide whether to just refer to it directly inside other functions or if it should be passed in as a parameter.
• I used 'first' quite a few times in the code to get the first value in a sequence but it doesn't feel like the code expresses the structure of the document very well.
• What's the best way to lay out code for 'defn' expressions? I've been putting the signature on it's own line and then the implementation on other lines which seems to be the way that it's done in the Clojure source code but it sometimes seems like I could just write it all on one line.

I recently came across an interesting blog post by Rod Hilton on unit testing and it reminded me of a couple of conversations Phil, Raph and I were having about the best way to test classes which delegate some responsibility to another class.

An example that we ran into recently was where we wrote some code which required one controller to delegate to another.

public class ControllerOne extends Controller {
    public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
    }
}

public class ControllerTwo extends Controller {
	private final ControllerOne controllerOne;
 
	public ControllerTwo(ControllerOne controllerOne) {
		this.controllerOne = controllerOne;
	}
 
    public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
		....
		return controllerOne.handleRequest(...);
    }
}

My initial thought when working out how to test this code was that we should check that the request is actually getting routed via ControllerOne:

@Test
public void theTest() {
	ControllerOne controller One = mock(ControllerOne.class);
 
	ControllerTwo controllerTwo = new ControllerTwo(controllerOne);
 
	controllerTwo.handleRequest(...)
 
	verify(controllerOne).handleRequest(...);
}

When we discussed this Raph and Phil both pointed out that we didn't care that specifically about the implementation of how the request was handled. What we care about is that the result we get after the request is handled is as expected.

We therefore changed our test to be more like this:

@Test
public void theTest() {
	ControllerOne controller One = mock(ControllerOne.class);
	ModelAndView myModelAndView = new ModelAndView();
	when(controllerOne.handleRequest(...).thenReturn(myModelAndView);
 
	ControllerTwo controllerTwo = new ControllerTwo(controllerOne);
 
	ModelAndView actualModelAndView = controllerTwo.handleRequest(...)
 
	assertThat(actualModelAndView, equalTo(myModelAndView));
}

I've been finding more and more recently that when it comes to writing tests which do some sort of delegation the 'stub + assert' approach seems to work out better than just verifying.

You lose the fine grained test that verifying mocks provides but we can still pretty much tell indirectly whether the dependency was called because if it wasn't then it's unlikely (but of course still possible) that we would have received the correct 'ModelAndView' in our assertion.

My current approach is that I'd probably only mock and verify an interaction if the dependency is a service which makes a network call or similarly expensive call where the interaction is as important as the result obtained.

For example we probably wouldn't want to make that call multiple times and with verification we're able to ensure that doesn't happen.

I find as I've used mocking frameworks more I feel like I'm drifting from a mockist style of testing to one getting closer to the classicist approach. I wonder if that's quite a normal progression.

In my continued playing around with Clojure I came across the 'apply' function which is used when we want to call another function with a number of arguments but have actually been given a single argument which contains the argument list.

The example that I've been trying to understand is applying 'str' to a collection of values.

I started off with the following:

(str [1 2 3])
=> "[1 2 3]"

This just returns the string representation of the vector that we passed it, but what we actually want is to get an output of "123″.

The 'apply' function allows us to do that:

(apply str [1 2 3])
=> "123"

That is semantically/conceptually the same as doing this:

(str 1 2 3)
=> "123"

I didn't quite understand how that could work though and my assumption was that somewhere in the Clojure source the above function call would be happening.

The definition of 'apply' is as follows:

(defn apply
  "Applies fn f to the argument list formed by prepending args to argseq."
  {:arglists '([f args* argseq])}
  [#^clojure.lang.IFn f & args]
    (. f (applyTo (spread args))))

The first thing which I hadn't realised is that when you have an '&' before a parameter definition then any arguments provided will be put into a list.

If we break down the example above we end up with the following:

(. str (applyTo (spread [[1 2 3]])))

The 'spread' function is defined like so:

1
2
3
4
5
6
7

(defn spread
  {:private true}
  [arglist]
  (cond
   (nil? arglist) nil
   (nil? (next arglist)) (seq (first arglist))
   :else (cons (first arglist) (spread (next arglist)))))

In this case we only have one item in 'arglist' so on line 6 the 'next arglist' expression evaluates to nil.

This means that we create a seq from the first argument of the 'arglist' which is '[1 2 3]'.

Working our way back up to the 'apply' function what we end up with is this:

(. str (applyTo (seq [1 2 3]))) 
=> "123"

This calls through to an 'applyTo' method defined on the 'clojure.lang.IFn' interface.

I'm not sure which of the implementations 'str' maps to but it seems like the 'str' function would eventually be called from the Java code with each of the values in the sequence passed in as a separate argument which is pretty neat!

In our latest technical book club we discussed chapter 10 – 'I Can't Run This Method in a Test Harness' – of Michael Feather's 'Working Effectively With Legacy Code'.

In this chapter Feathers outlines some of the problems we might have getting methods under test and then suggests some ways to get around those problems.

These are some of my thoughts and our discussion of the chapter:

• I quite like the idea of pragmatic refactoring that Feathers suggests early on in the chapter:
  

  Ideally, it would be great to break it down into smaller classes, but we have to carefully consider whether we want to do that much refactoring right now. It would be great to do it, but whether we can depends on where we are in our release cycle, how much time we have, and all the associated risks.

  I think it's important to understand when hacking code in is going to hurt us and quite often I think the side effects of taking this approach are underestimated.

  Tom spoke of 'value fetishism' whereby we get so caught up trying to add 'business value' that we forget to keep the code in good stead for future work.

  I quite like the analogy that Alistair Cockburn uses of software development as a series of cooperative gamem and I think it's sometimes easy to forget in the rush to get some code out for a release that we have to make sure that we don't make our lives impossible for the next game/release by rushing code in without paying due attention.

• Feathers spends some time suggesting how we can test private methods – one way is to change the method to be public. There are a couple of reasons why we might not want to do that:
  • The method is just a utility and isn't something that clients would care about. We have therefore made our API more noisy
  • If a client calls the method then it could have an adverse affect on the results of other methods in the class

  The cleanest solution for that second point is to move those methods out into their own class where we could test against them directly. A smaller step to take is to make the method protected and then create a test version of the class from which we can call the protected method directly.

  Feathers also talks about the danger of using reflection to allow us to test this kind of code – it might allow us to get tests around the code but we're not making the code any better and we may not notice quite how bad the code has got. We are just delaying the inevitable.

  There's a tool in .NET land called TypeMock which allows you to test pretty much anything but I wonder whether we would run into the same problems that Feathers describes above.

• I quite liked the skin and wrap the API pattern whereby we create our own interface for a tricky dependency. We then refer to the interface in our code instead of the concrete dependency and have a class which just delegates to the real dependency. We did this when trying to test file operations on a project I worked on a couple of years ago.

A function that we had to write in Java on a project that I worked on recently needed to indicate whether there was a gap in a series of data points or not.

If there were gaps at the beginning or end of the sequence then that was fine but gaps in the middle of the sequence were not.

null, 1, 2, 3 => no gaps
1, 2, 3, null => no gaps
1, null, 2, 3 => gaps

The Java version looked a bit like this:

public boolean hasGaps(List<BigInteger> values) {
    Iterator<BigInteger> fromHead = values.iterator();
    while (fromHead.hasNext() && fromHead.next() == null) {
        fromHead.remove();
    }
 
    Collections.reverse(values);
 
    Iterator<BigInteger> fromTail = values.iterator();
    while (fromTail.hasNext() && fromTail.next() == null) {
        fromTail.remove();
    }
 
    return values.contains(null);
}

We take the initial list and then remove all the null values from the beginning of it, then reverse the list and remove all the values from the end.

We then check if there's a null value and if there is then it would indicate there is indeed a gap in the list.

To write this function in Clojure we can start off by using the 'drop-while' function to get rid of the trailing nil values.

I started off with this attempt:

(defn has-gaps? [list]
    let [no-nils] [drop-while #(= % nil) list]
  no-nils)

Unfortunately that gives us the following error!

Can't take value of a macro: #'clojure.core/let (NO_SOURCE_FILE:16)

It thinks we're trying to pass around the 'let' macro instead of evaluating it – I forgot to put in the brackets around the 'let'!

I fixed that with this next version:

(defn has-gaps? [list]
    (let [no-nils] [drop-while nil? list]
  no-nils))

But again, no love:

java.lang.IllegalArgumentException: let requires an even number of forms in binding vector (NO_SOURCE_FILE:23)

The way I understand it the 'let' macro takes in a vector of bindings as its first argument and what I've done here is pass in two vectors instead of one.

In the bindings vector we need to ensure that there are an even number of forms so that each symbol can be bound to an expression.

I fixed this by putting the two vectors defined above into another vector:

(defn has-gaps? [list]
    (let [[no-nils] [(drop-while nil? list)]]
  no-nils))

We can simplify that further so that we don't have nested vectors:

(defn has-gaps? [list]
    (let [no-nils (drop-while nil? list)]
  no-nils))

The next step was to make 'no-nils' a function so that I could make use of that function when the list was reversed as well:

(defn has-gaps? [list]
    (let [no-nils (fn [x] (drop-while nil? x))]
  (no-nils list)))

I then wrote the rest of the function to reverse the list and then check the remaining list for nil:

(defn has-gaps? [list]
    (let [[no-nils] [(fn [x] (drop-while nil? x))]
          [nils-removed] [(fn [x] ((comp no-nils reverse no-nils) x))]]
  (some nil? (nils-removed list))))

The 'comp' function can be used to compose a set of functions which is what I needed.

It seemed like the 'nils-removed' function wasn't really necessary so I inlined that:

(defn has-gaps? [list]
    (let [no-nils (fn [x] (drop-while nil? x))]
  (some nil? ((comp no-nils reverse no-nils) list))))

The function can now be used like this:

user=> (has-gaps? '(1 2 3))
nil
user=> (has-gaps? '(nil 1 2 3))
nil
user=> (has-gaps? '(1 2 3 nil))
nil
user=> (has-gaps? '(1 2 nil 3))
true

I'd be intrigued to know if there's a better way to do this.

Something which an agile approach on a project typically gives us is the ability to change requirements rapidly based on the different types of feedback we typically get over the course of the project.

One way that we can lose this advantage is by getting caught up by the number of story points being completed and using this as the measure of success.

The flexibility to change has an impact on the number of story points that may be completed in a given iteration – if we start doing some work on a story and then get feedback from the business while it is still in progress it's possible that we will end up with more work to do than we had previously.

Once a story is estimated we don't typically go back and change that estimate unless there was a change in the assumptions being made before we play it. This means that any changes we make based on feedback would not result in re-estimation of the story card.

We now have more work to do without our measurement mechanism recognising that.

It gets even worse if we start working on a feature and then get feedback from the business that the feature is no longer useful to them and we should just stop working on it.

At this point a fairly common response is to 'lock down' the requirements so that we'll only work on the currently defined requirements and no new ones or changes will be permitted.

While this might make some logical sense it misses the point that we're supposed to try and deliver something that is actually useful rather than something which just meets the initial requirements.

In reality one of two things, or perhaps a combination of both, ends up happening:

• There will still be changes and all we've done is create the illusion that there won't be any.
• The business will now push to get exactly the features that were specified originally even if the development team ends up spending a disproportionate amount of time creating those features for the value they actually provide.

Story points are really useful for allowing us to get an idea of the relative size of stories and can be helpful for getting a rough understanding of the amount of work we can complete in a given time period but we need to be careful how we use them.

Our goal at the end of the day is to deliver working software.

Story points are just a mechanism for giving us some feedback on how well we are doing that. They're not the goal in itself.

Dan North has been presenting his 'Pimp my architecture' talk again at QCon San Francisco this week and after reading the hugely positive feedback on Twitter I decided to watch some of it again.

The idea of getting people to help each other rather than pair program is what stood out for me this time, something which Brian Guthrie also pointed out:

"We didn't do pairing, we did 'helping'. You can't get alpha progs to 'pair' but they'll tell you what they know."

It's been quite common at the places that I've worked at for pair programming to be frowned upon and sometimes that's not the most important battle to fight so the team will be more selective about its use.

I was discussing this with some colleagues this week and it does seem that pair programming is a trigger word/phrase with negative connotations for productivity.

On the other hand, something which is rarely frowned upon is having a team 'working collaboratively' and in fact most of the time this is considered a very good thing.

When we work alone I still find myself calling a colleague over to help me out when I get stuck and while I could probably work something out alone given enough time I don't think that approach makes sense when working on it with someone else leads to us solving the problem much more quickly.

Josh Bloch speaks along similar lines in the interview with him in Coders at Work:

I don't like working in total isolation. When I'm writing a program and I come to a tricky design decision, I just have to bounce it off someone else. At every place I've worked, I've had one or more colleagues I could bounce ideas off of. That's critically important for me; I need that feedback.
…
I've known people who don't feel this way – who are willing to program in a vacuum. I think it hurts them.

I think it's fairly inevitable that if a team is to deliver software effectively then there are going to be times when people are working together to solve problems.

What we choose to call that can be varied depending on the context we find ourself in.

Something which I wanted to do recently was write a function that would indicate whether a collection contained a nil value.

I initially incorrectly thought the 'contains?' function was the one that I wanted:

(contains? '(1 nil 2 3) nil)
=> false

I thought it would work the same as the Java equivalent but that function actually checks whether a key exists in a collection rather than a value. It's more useful when dealing with maps.

There's more discussion on the consistency of the API on the mailing list.

Luckily the documentation guides us towards the 'some' function:

My first attempt was to write an anonymous function to check if there was a 'nil' in the list:

(some #(= % nil) '(1 nil 2 3))
=> true

(some #(= % nil) '(1  2 3))
=> nil

fogus showed me an even better way by making use of the built in 'nil?' function:

(some nil? '(1 nil 2 3))

Another approach would be to make use of the Java 'contains' method as Philip Schwarz pointed out:

(.contains '(1 nil 2 3) nil)
=> true

I noticed that when you use Java methods in Clojure with collections then the result will either be 'true' or 'false' whereas when you use Clojure built in functions then it's more likely to be 'true' or 'nil'.

I guess this is linked to the idea that 'nil' is false in Clojure so it doesn't make much difference what the return value is.

When I'm using a language I've got into the habit of just trying out the API in the way that I expect it to work rather than paying a lot of attention to what the API documentation says.

I think this is something I'll need to work out to avoid much frustration!

In my continued playing with Clojure I'm noticing a few things that I keep getting confused about.

The meaning of parentheses

Much like Keith Bennett I'm not used to parentheses playing such an important role in the way that an expression gets evaluated.

As I understand it if an expression is enclosed in parentheses then that means it will be evaluated as a function.

For example I spent quite a while trying to work out why the following code kept throwing a class cast exception:

(if (true) 1 0)

If you run that code in the REPL you'll get the following exception because 'true' isn't a function and therefore can't be applied as such:

java.lang.ClassCastException: java.lang.Boolean cannot be cast to clojure.lang.IFn (NO_SOURCE_FILE:0)

If we don't want something to be treated this way then the parentheses need to disappear!

(if true 1 0)

Truthyness

Somewhat related to the above is understanding which expressions evaluate to 'true' or 'false'.

I'm told there are some edge cases but that as a general rule everything except for 'false' and 'nil' evaluates to true.

I think that's an idea which is more common in languages like Ruby but I'm not yet used to the idea that we can something like this and have it execute:

(if "mark" 1 0)

In C# or Java I would except to have to compare "mark" to something in order for it to evaluate to a boolean result.

It seems like a really neat way to reduce the amount of code we have to write though so I like it so far.

Character Literals

I've been working through Mark Volkmann's Clojure tutorial and in one example he defines the following function:

(def vowel? (set "aeiou"))

I wanted to try it out to see if a certain character was a vowel so I initially did this:

=>(vowel? "a")
nil

"a" is actually a string though which means it's an array of characters when what we really want is a single character.

I thought the following would be what I wanted:

(vowel? 'a')

Instead what I got was the following exception:

java.lang.Exception: Unmatched delimiter: )

This one just turned out to be a case of me not reading the manual very carefully and actually the following is what I wanted:

=> (vowel? \a)
\a

An interesting object orientation related problem that Raph and I were looking at recently revolved around the design of two controllers in the application we've been working on.

The two controllers in question look roughly like this:

public class GenericController extends Controller {
	private final SomeFactory someFactory;
 
	public GenericController(SomeFactory someFactory);
        this.someFactory = someFactory;
	}
 
    public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
        // do some stuff but never use 'request' or 'response'
    }
}

public class MoreSpecificController extends GenericController {
	private final SomeFactory someFactory;
 
	public MoreSpecificController(SomeFactory someFactory);
        super(someFactory);
	}
 
    public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception {
    		...    
		// do some stuff which does use 'request'
		String someValue = request.getParameter("someParameter");
    }
}

We noticed from the way that we wrote tests for these two classes that we seem to be breaking the principle of type conformance as defined in Meilir Page Jones' 'Fundamentals of Object Oriented Design in UML' and more commonly referred to as the Liskov Substitution Principle.

The principle states:

If S is a true subtype of T, then S must conform to T. In other words, an object of type S can be provided in any context where an object of T is expected and correctness is still preserved when any accessor operation of the object is executed

This means that wherever we make use of 'GenericController' in our code it should be possible to pass in an instance of 'MoreSpecificController' and it would adhere to the same contract.

Our tests for each of the controllers looked a bit like this:

@Test
public void someTest() {
	GenericController genericController = new GenericController();
 
	genericController.handleRequest(null, null);
 
	// and so on
}

@Test
public void someTest() {
	MoreSpecificController moreSpecificController = new MoreSpecificController();
 
	moreSpecificController.handleRequest(mock(HttpServletRequest.class), mock(HttpServletResponse.class));
 
	// and so on
}

In 'MoreSpecificController' we need to get a value from the request which means that we can't have it as null in the test. In 'GenericController' the request is actually irrelevant so we can pass in a null value.

This means that the pre condition for 'MoreSpecificController' is stronger than the pre condition for 'GenericController' which violates the principle of contravariance which states the following:

Every operation's precondition is no stronger than the corresponding operation in the superclass. The strength of operation preconditions in the subclass goes in the opposite direction to the strength of the class invariant. That is, the operations preconditions get, if anything, weaker

The reason this might cause a problem is because if a client had a reference to a 'GenericController' they should expect that they can treat an instance of 'MoreSpecificController' as if it was a 'GenericController' which should mean that we can pass null values for request and response.

We weren't ever referring to a 'GenericController' when we instantiated a 'MoreSpecificController' in our code base so it didn't prove to be a problem but in theory it seems like something we'd want to avoid.