Mark Needham

I’ve been playing around with more F# code and after realising that I’d peppered the code with if statements I thought it would be interesting to try and refactor it to make use of active patterns.

The code is part of my F# solution to Roy Osherove’s TDD Kata and is used to parse the input string and find which delimeters are being used.

This is the original code:

    let hasCustomDelimeter (value:string) = value.Length > 2 && "//".Equals(value.Substring(0, 2))
    let hasMultipleDelimeters (value:string) = hasCustomDelimeter value && "[".Equals(value.Substring(2,1))
 
    let delimeter value = if (hasCustomDelimeter value) then value.Substring(2, value.IndexOf("\n") - 2) else ","
    let delimeters (value:string) = Regex.Matches(value, "\[(.)\]") |> 
                                    Seq.cast |> 
                                    Seq.map (fun (x:Match) -> x.Groups) |>
                                    Seq.map (fun x -> x |> Seq.cast<Group> |> Seq.nth 1) |>
                                    Seq.map (fun x -> x.Value) |>
                                    Seq.to_array   
 
    let digits value = 
        let delimeter = delimeter value
        if (hasMultipleDelimeters value) then value.Substring(value.IndexOf("\n")) |> split (delimeters value) |> Array.map parse  
        else if (hasCustomDelimeter value) then value.Substring(value.IndexOf("\n")) |> split [| delimeter |] |> Array.map parse
        else value.Replace("\n", delimeter) |> split [|delimeter |] |> Array.map parse

The active pattern that we want to use is known as a ‘multi case active pattern’ which Chris Smith defines as “partitioning the entirety of the input space into different things”. In this case given an input string we want to determine what type of delimeters are contained within that string.

This is the code after I’d created the active pattern:

    let (|MultipleCustomDelimeters|SingleCustomDelimeter|NoCustomDelimeter|) (value:string) =  
        if (value.Length > 2 && "//".Equals(value.Substring(0, 2))) then
            if ("[".Equals(value.Substring(2,1))) then MultipleCustomDelimeters(delimeters value)
            else SingleCustomDelimeter(delimeter value)
        else NoCustomDelimeter(delimeter value)    
 
    let digits value =  
        match value with 
        | SingleCustomDelimeter(delimeter)     -> value.Substring(value.IndexOf("\n")) |> split [| delimeter |] |> Array.map parse
        | MultipleCustomDelimeters(delimeters) -> value.Substring(value.IndexOf("\n")) |> split delimeters  |> Array.map parse 
        | NoCustomDelimeter(delimeter)         -> value.Replace("\n", delimeter) |> split [|delimeter |] |> Array.map parse

One thing I didn’t realise is that you can set different types for the constructor values of each of the active patterns. In this case I want to match single or multiple delimeters and then return those delimeters. We can define one active pattern which matches a single delimeter and just returns that and then another one which returns an array of delimeters which is quite neat.

The way it works is quite similar to discriminated unions.

I found that that having all the code around parsing the input in the same function made it easier for me to understand that code and I quite like that it’s possible to match the pattern and also get the delimeter/delimeters in one expression.

Although there are more lines of code I think this code is more expressive and it wouldn’t be too hard to add in another active pattern if there’s another delimeter type that needs to be handled.

I can’t decide whether the ‘value.SubString’ and ‘value.Replace’ code should also be contained within the active pattern or not. At the moment I’m thinking perhaps not because it’s not related to the actual delimeters.

A fairly common discussion that I’ve had with several of my colleagues is around the way that we name the variables used for mocks and stubs in our tests.

There seems to be about a 50/50 split between including ‘Stub’ or ‘Mock’ on the end of those variable names and not doing so.

In a simple example test using Rhino Mocks as the testing framework this would be the contrast between the two approaches:

[Test]
public void ShouldDoSomething()
{
	var someDependency = MockRepository.CreateMock<ISomeDependency>();
 
	someDependency.Expect(x => x.SomeMethod()).Return("someValue");
 
	var myController = new MyController(someDependency);
        myController.DoSomething()
 
	someDependency.VerifyAllExpectations();
}

[Test]
public void ShouldDoSomething()
{
	var someDependencyMock = MockRepository.CreateMock<ISomeDependency>();
 
	someDependencyMock.Expect(x => x.SomeMethod()).Return("someValue");
 
	var myController = new MyController(someDependencyMock);
        myController.DoSomething()
 
	someDependencyMock.VerifyAllExpectations();
}

I favour the former where we don’t specify this information because I think it adds unnecessary noise to the name and is a detail about the implementation of the object behind that variable which I don’t care about when I’m reading the test.

I do care about it if I want to change something but if that’s the case then I can easily see that it’s a mock or stub by looking at the place where it’s instantiated.

From my experience we often tend to end up with the situation where the variable name suggests that something is a mock or stub and then it’s used in a different way:

[Test]
public void ShouldDoSomething()
{
	var someDependencyMock = MockRepository.CreateMock<ISomeDependency>();
 
	someDependencyMock.Stub(x => x.SomeMethod()).Return("someValue");
 
	var myController = new MyController(someDependencyMock);
        myController.DoSomething()
}

That then becomes a pretty misleading test because the reader is unsure whether the name is correct and the stub call is incorrect or whether it should in fact be a stub and the name is wrong.

The one time that I’ve seen that extra information being useful is when we have really long tests – perhaps when writing tests around legacy code which is tricky to get under test.

In this situation it is very nice to be able to easily see exactly what we’re doing with each of our dependencies.

Hopefully this is only a temporary situation before we can work out how to write simpler tests which don’t require this extra information.

In my continued efforts at Roy Osherove’s TDD Kata I’ve been trying to work out how to split a string based on a delimeter which contains more than one character.

My original thinking was that it should be possible to do so like this:

"1***2".Split("***".ToCharArray());;

I didn’t realise that splitting the string like that splits on each of the stars individually which means that we end up getting 2 empty values in the result:

val it : string [] = [|"1"; ""; ""; "2"|]

If we want to split on ‘***’ then we have to pass it in as a value in a string array:

"1***2".Split([| "***" |], StringSplitOptions.None);;

That way we only get the 1 and 2 which is what we want:

val it : string [] = [|"1"; "2"|]

I’d expected there to be an overload which takes in a string and then just splits on that but since there isn’t this isn’t a bad alternative.

Sam Allen has a very interesting article which covers all sorts of way to split different types of strings.

A while ago I wrote about F#’s forward and application operators where I’d looked at how these could be used to simplify code and while trying out Roy Osherove’s TDD Kata I realised that perhaps the choice of which of these to use or whether to use them at all depends on what intent we’re expressing.

The specific bit of code I was writing was for raising an exception if negative values were provided and I originally thought I’d use the forward operator to express this code:

    let digits = [| 1;2;3;-3 |] 
    let buildExceptionMessage negatives = sprintf "No negative numbers allowed. You provided %s" 
                                                  (String.Join(",", negatives |> Array.map (fun x -> x.ToString())))
 
    raise (ArgumentException (digits |> Array.filter (fun x -> x < 0) |> buildExceptionMessage))

I think in this case the forward operator doesn’t actually express the intent of the code better because it puts the focus on the digits rather than on the building of the exception message.

I changed that a bit to emphasise the importance of the ‘buildExceptionMessage’ function:

raise (ArgumentException (buildExceptionMessage (digits |> Array.filter (fun x -> x < 0))))

I thought it might be possible to get rid of the brackets around the filtering of the digits and instead apply that expression to ‘buildExceptionMessage’ using the application operator:

raise (ArgumentException (buildExceptionMessage <| digits |> Array.filter (fun x -> x < 0)))

That actually results in the following error message:

Type mismatch. Expecting a  string -> string but given a  'a array -> 'a array. The type 'string' does not match the type ''a array'

The problem is that it applies digits to ‘buildExceptionMessage’ first and then tries to apply the result of that to Array.filter instead of applying the filter to the digits and then passing the result of that calculation to ‘buildExceptionMessage’.

One way to get around this is to remove the forward operator and move digits to be the second argument passed to Array.filter instead:

raise (ArgumentException (buildExceptionMessage <| Array.filter (fun x -> x < 0) digits))

This is the version that I’ve got at the moment and I think it expresses the intent of the code the best.

I’d be interested in hearing more thoughts on the best way to use or not use these operators in idiomatic F# code.

I recently watched Randy Pausch’s ‘Last Lecture: Achieving Your Childhood Dreams‘ and read the corresponding book and although it’s not directly related to software development I think that some of the points that he makes are really intriguing.

These were some of the parts that particularly stood out for me:

• Introduce the elephant in the room – whatever it is that people are really thinking about, put it out in the open. I think on development teams there is often a distrust of the way that other people write code because it’s different to the way that we do. If we can get this out in the open more frequently and agree on a shared approach then it should result in a more consistent code base.
• Get the fundamentals down – Pausch suggests that we need to understand the fundamentals because otherwise the fancy stuff isn’t going to work. In Apprenticeship Patterns Ade Oshineye and Dave Hoover suggest that we should ‘Study the Classics‘ which is a similar idea.

  While I think they’re certainly right I’m not sure that learning theory before putting it into practice is the most effective way to learn. I think we need to do a bit of both at the same time alternating between the two so that we actually have a frame of reference when we’re learning the fundamentals.

• The brick walls are there for a reason – Pausch describes how we’ll often come across obstacles stopping or blocking us from what we want to do but that we shouldn’t be discouraged, they are there so that we can see how much we really want something. I think this is just generally true and not just related to software development.
• The skill of self assessment – Pausch suggests that we need know what we don’t know which I think is perhaps the best advice in the book. We need to recognise our true abilities but also have a sense of our flaws and create feedback loops to allow this to happen.

  My colleague Liz Douglass has come up with the idea of Feedback Frenzies to help create this feedback loop and this is one of the best ideas I’ve come across for doing this.

• One of my favourite quotes from the book is the following which I believe was originally from Dan Stanford:
  

  Experience is what you get when you didn’t get what you wanted

  …

  It’s a phrase worth considering at every brick wall we encounter, and at every disappointment. It’s also a reminder that failure is not just acceptable, it’s often essential.

  I think we can do this much more in the software world. There sometimes seems to be a stigma with identifying things which fail but I think it’s really useful for others to learn from mistakes that have been made.

The Last Lecture is one of the best presentations I’ve had the chance to watch – I’d certainly recommend it.