Mark Needham

Something which I’ve noticed quite a lot on the projects that I’ve worked on since C# 3.0 was released is that lists seem to be passed around code much more and have LINQ style filters and transformations performed on them while failing to describe the underlying abstraction explcitly in the code.

As a result of this we quite frequently we end up with this code being in multiple places and since it’s usually not very much code the repetition goes unnoticed more than other types of duplication might do.

A typical example of this might be the following:

public class SomeFooHolder
{
	public List<Foo> Foos { get; set }
}

An example of how this might be used is like so:

var someFooHolder = new FooHolder(...);
someFooHolder.Foos.Select(f => f.Completed);

That code would typically be repeated in other places in the code where we want to get all the completed foos.

Although it’s a simple change, as a first step I prefer to make that concept more explicit by putting ‘CompletedFoos’ on ‘SomeFooHolder’:

public class SomeFooHolder
{
	public List<Foo> Foos { get; set; }
	public List<Foo> CompletedFoos 
	{ 
		get { return Foos.Select(f => f.Completed); }
	}
}

Perhaps an even better solution would be to create an object ‘Foos’ to encapsulate that logic further:

public class Foos
{
	private readonly List<Foo> foos;
 
	public Foos(List<Foo> foos)
	{
		this.foos = new List<Foo>(foos.AsReadOnly());
	}
 
	public Foos Completed
	{
		get { return new Foos(foos.Select(f => f.Completed)); }
	}
}

As I’ve written about previously I prefer to wrap the list rather than extend it as the API of ‘Foos’ is more expressive since we don’t have all the list operations available to any potential users of the class.

One of the common tasks that we need to do on every project I’ve worked on is ensure that we can create a web.config file for the different environments that we need to deploy our application to.

Nant has quite a neat task called ‘expandproperties‘ which allows us to do this quite easily.

In our build file we would have the following:

build-file.build

  <property name ="configFile" value="${environment}.properties" readonly="true"/>
  <if test="${not file::exists(configFile)}">
    <fail message="Configuration file '${configFile}' could not be found." />
  </if>
  <include buildfile ="${configFile}" />
 
  <target name="GenerateConfigFiles">
    <foreach item="File" property="TemplateFile">
      <in>
        <items>
          <include name="ProjectDirectory/**.template"/>
        </items>
      </in>
      <do>
        <copy file="${TemplateFile}" tofile="${string::replace(TemplateFile,'.template','')}" overwrite="true">
          <filterchain>
            <expandproperties />
          </filterchain>
        </copy>
      </do>
    </foreach>
  </target>

There would be corresponding ‘.template’ and ‘.properties’ files containing the various variables we want to set, like so:

dev.properties

 <project>
	 <property name="SomeHost" value="http://my-host.com:8080"/>
 </project>

web.template.config

<?xml version="1.0"?>
...
<configuration>
   <appSettings>
      <add key="SomeHost="${SomeHost}"/>
   </appSettings>
</configuration>
...

We would call the build file like so:

nant -buildfile:build-file.build GenerateConfigFiles -D:environment=dev

We can then change that ‘environment’ variable depending which one we need to generate the configuration files for.

We were working on some refactoring today where we pushed some logic back from a service and onto a domain object and I noticed that we were able to use functions quite effectively to reduce the amount of code we had to write while still describing differences in behaviour.

The class we want to write needs to take in two integers which represent two different situations related to Foo. Depending upon whether we have ‘Situation 1′, ‘Situation 2′ or both situations we will display the results slightly differently.

    public class Foo
    {
        private readonly int situationOneValue;
        private readonly int situationTwoValue;
        private readonly Func<int, string> situationOneDisplayFunc;
        private readonly Func<int, string> situationTwoDisplayFunc;
 
        private Foo(int situationOneValue, int situationTwoValue, Func<int, string> situationOneDisplayFunc, Func<int, string> situationTwoDisplayFunc)
        {
            this.situationOneValue = situationOneValue;
            this.situationTwoValue = situationTwoValue;
            this.situationOneDisplayFunc = situationOneDisplayFunc;
            this.situationTwoDisplayFunc = situationTwoDisplayFunc;
        }
 
        public static Foo ForSituation1(int situationOneValue, int situationTwoValue)
        {
            return new Foo(situationOneValue, situationTwoValue, Format, _ => "Irrelevant value");
        }
 
        public static Foo ForSituation2(int situationOneValue, int situationTwoValue)
        {
            return new Foo(situationOneValue, situationTwoValue, _ => "Irrelevant value", Format);
        }
 
        public static Foo ForSituation1And2(int situationOneValue, int situationTwoValue)
        {
            return new Foo(situationOneValue, situationTwoValue, Format, Format);
        }
 
        public string Situation1()
        {
            return situationOneDisplayFunc(situationOneValue);
        }
 
        public string Situation2()
        {
            return situationTwoDisplayFunc(situationTwoValue);
        }
 
        private string Format(int value)
        {
            return string.Format("Formatted Value {0}");
        }
    }

The way that it’s used is that we get a value coming in from another system as a string which represents which situation we have to deal with.

As a result in the code which processes this data we have a dictionary which maps from these strings to the corresponding static method defined above:

private static Dictionary<string, Func<int, int, Foo>> Foos = 
    new Dictionary<string, Func<int, int, Foo>> 
    { { "Situation 1", Foo.ForSituation1 }, 
      { "Situation 2", Foo.ForSituation2 }.
      { "Situation 1 and 2", Foo.ForSituation1And2 };

This is a neat little idea which I learnt from some Scala code that my colleague John Hume posted on an internal mailing list a few weeks ago where he’d done something similar.

In our code which parses the incoming data we have something like the following:

var foo = Foos[incomingMessage.Situation](incomingMessage.Situation1Value, incomingMessage.Situation2Value);
// and so on

If we wanted to solve this without using functions then for the first part I think we would probably need to use inheritance to create three different Foos, probably with a common interface, and then define the different behaviour on those classes.

The second part of the solution might end up being solved with some if statements or something similar.

I was a bit unsure of how easy it was to understand this code when we first came up with it but it’s growing on me.

A few months ago Uncle Bob wrote a post about TDD where he suggested that he preferred to use hand created stubs in his tests wherever possible and only resorted to using a Mockito created stub as a last resort.

I wrote previously about my thoughts of where to use each of the two approaches and one example of where hand written stubs seems to make sense is the clock.

I wonder if this ties in with J.B. Rainsberger’s theory of irrelevant details in the tests which make use of it.

We would typically define an interface and stub version of the clock like so:

public interface IClock
{
	DateTime Now();
}

public class ControlledClock : IClock
{
	private readonly DateTime dateTime;
 
	public ControlledClock(DateTime dateTime)
	{
		this.dateTime = dateTime;
	}
 
	public DateTime Now() 
	{ 
		return dateTime; 
	}
}

I forgot about it to start with and was stubbing it out using Rhino Mocks but I realised that every single test needed something similar to the following code:

1
2
3

var theCurrentTime = new DateTime(2010, 1, 16);
var clock = MockRepository.GenerateStub<IClock>();
clock.Stub(c => c.Now()).Return(theCurrentTime);

We can extract out the creation of the first two lines into fields but the third line remains and typically that might end up being pushed into a setup method which is run before each test.

With a clock it’s maybe not such a big deal but with other dependencies from my experience it can become very difficult to follow where exactly the various return values are being setup.

When we use a hand written stub we only have to write the following code and then the date is controlled everywhere that calls ‘Now()’:

private readonly IClock clock = new ControlledClock(new DateTime(2010,1,16));

Following on from that my colleague Mike Wagg suggested the idea of creating extension methods on integers to allow us to fluently define values relative to the clock.

[Test]
public void ShouldShowFoosOlderThanToday()
{
	var clock = new ControlledClock(new DateTime(2010,1,16));
	var fooService = MockRepository.GenerateStub<IFooService>();
 
	var fooFromYesterday = new Foo { Date = 1.DayBefore(clock) });
	var aCollectionOfFoos = new List<Foo> { fooFromYesterday };
	fooService.Stub(f => f.GetFoos()).Return(aCollectionOfFoos);
 
	var oldFoos = new FooFinder(clock, fooService).GetFoosFromEarlierThanToday();
 
	Assert.That(oldFoos.Count, Is.EqualTo(1));
	// and so on
}

The extension method on integer would be like this:

public static class IntegerExtensions
{
	public static DateTime DayBefore(this int value, IClock clock)
	{
		return clock.Now.Subtract(TimeSpan.FromDays(value));
	}
}

It reads pretty well and seems more intention revealing than any other approaches I’ve tried out so I think I’ll be continuing to use this approach.

A few months ago Uncle Bob wrote a post about TDD where he suggested that he preferred to use hand created stubs in his tests wherever possible and only resorted to using a Mockito created stub as a last resort.

I’ve tended to use framework created ones but my colleague Matt Dunn and I noticed that it didn’t seem to work out too well for us writing some tests around a controller where the majority of our tests were making exactly the same call to that repository and expected to receive the same return value but a few select edge cases expected something different.

The edge cases came along later on by which time we’d already gone past the stage of putting the stub expectation setup in every test and had moved it up into a setup method which ran before all test.

We tried to keep on using the same stub instance of the dependency which meant that we were trying to setup more than one stub expectation on the same method with different return values.

[TestFixture]
public SomeControllerTests
{
	private ITheDependency theDependency;
 
	[SetUp]
	public void Before()
	{
		theDependency = MockRepository.GenerateStub<ITheDependency>();
		theDependency.Stub(t => t.SomeMethod()).Return(someResult);
 
		controller = new Controller(theDependency);
	}
 
	....
 
	[Test]
	public void SomeAnnoyingEdgeCaseTest()
	{
		theDependency.Stub(t => t.SomeMethod().Return(someOtherResult);
 
		controller.DoSomething();
 
		// and so on...
	}
 
}

We were struggling to remember where we had setup the various return values and could see a few ways to try and reduce this pain.

The first was to extract those edge case tests out into another test fixture where we would setup the stub expectation on a per test basis instead of using a blanket approach for all of them.

This generally works pretty well although it means that we now have our tests in more than one place which can be a bit annoying unless you know that’s what’s being done.

Another way which Matt suggested is to make use of a hand written stub for the tests which aren’t bothered about the return value and then use a framework created one for specific cases.

The added benefit of that is that it’s more obvious that the latter tests care about the result returned because it’s explicitly stated in the body of the test.

We found that our tests were easier to read once we’d done this and we spent much less time trying to work out what was going on.

I think framework generated stubs do have thier place though and as Colin Goudie points out in the comments on Uncle Bob’s post it probably makes more sense to make use of a framework generated stub when each of our tests returns different values for the same method call.

If we don’t do that then we end up writing lots of different hand written stubs which I don’t think adds much value.

I still start out with a framework generated stub but if it’s becoming overly complicated or repetitive then I’m happy to switch to a hand written one.

It’d be interesting to hear others’ approaches on this.

Since I started playing around with F# one of the things I’ve been trying to do is not use the ‘for’ keyword because I was trying to avoid writing code in an imperative way and for loops are a big part of this for me.

Having read Jon Harrop’s solution to the word count problem where he made use of both sequence and for expressions I thought it’d be intersting to see what some of the code I’ve written would look like using that approach.

An example of a function that I wrote which could be rewritten the other way is the following:

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)

This could be written like this if we used a sequence expression instead of chaining map operations:

let delimeters (value:string) = seq { for m in Regex.Matches(value, "\[([^]]*)\]") do yield m.Groups.Item(0).Value }

One interesting thing I found about writing it like this was that I noticed that ‘GroupCollection’ had the ‘Item’ property on it which would let me get the match much more easily.

I completely missed that when I was writing the first solution so I’m not sure if that was just due to my lack of knowledge of that part of the API or whether the second approach actually encouraged me to explore more and therefore end up with a simpler solution.

Another example I found was this expression for getting the matches for a regular expression:

let regex pattern input = Regex.Matches(input, pattern) |> Seq.cast |> Seq.map (fun (x:Match) -> x.Value)

That can be simplified to the following with a sequence expression:

let regex pattern input = seq { for m in Regex.Matches(input, pattern) do yield m.Value }

One neat thing about using sequence expressions is that we don’t need to make use of ‘Seq.cast’ to convert a value to a typed sequence – we can just use it as it is.

The following function can be rewritten to just use a for expression:

let writeTo (path:string) (values:seq<string * int>) = 
    use writer = new StreamWriter(path)
    values |> Seq.map (fun (value, count) -> value + " " + count.ToString()) |> Seq.iter (fun x -> writer.WriteLine(x))

Like so:

let writeTo (path:string) (values:seq<string * int>) = 
    use writer = new StreamWriter(path)
    for (value,count) in values do writer.WriteLine(value + " " + count.ToString())

We eventually iterate through the sequence anyway so I think it’s more intention revealing to just do the iteration and mapping in one step.

This is a function from when I was writing the little Feedburner application:

let calculateWeeklyAverages =
    Seq.reverseSequence >>
    Seq.windowed days >>
    Seq.map (fun (entries:array<Entry>) -> 
                (entries.[0]).Date , entries |> Array.map (fun e -> e.Circulation |> toDecimal) |> Array.average ) >>
    Seq.reverseSequence

If we use a sequence expression it’d look like this:

let calculateWeeklyAverages entries =
    seq { for (e:array<Entry>) in (entries |> Seq.reverseSequence |> Seq.windowed days) do 
              yield ((e.[0]).Date, entries |> Array.map (fun e -> e.Circulation |> toDecimal) |> Array.average) } 
    |> Seq.reverseSequence

The resulting code is shorter but it seems to me like the focus when you read the code has moved to the line which yields the tuple whereas in the first version I find that I read the function as a whole.

I’ve not really used sequence expressions that much so it’s been interesting going through the code and seeing where they might be useful.

I found several places where I’d used lists because I find those easier to pattern match against but I wonder whether it would make sense to use sequences there as well.

I’ve written previously about the test builder pattern in C# and having noticed some different implementations of this pattern I thought it’d be interesting to post my current thinking on how to use it.

One thing I’ve noticed is that we often end up just creating methods which effectively act as setters rather than easing the construction of an object.

This seems to happen most commonly when the value we want to set is a boolean value. The following is quite typical:

public CarBuilder WithSomething(boolean value)
{
	this.something = value;
	return this;
}

The usage would be like so:

new CarBuilder().WithSomething(true).Build();

It doesn’t read too badly but it seems to be unnecessary typing for the user of the API. If we’re going to use the fluent interface approach then I would prefer to have two methods, defined like so:

public CarBuilder WithSomething()
{
	this.something = true;
	return this;
}
 
public CarBuilder WithoutSomething()
{
	this.something = false;
	return this;
}

We could then use this like so:

new CarBuilder().WithSomething().Build();
...
new CarBuilder().WithoutSomething().Build();

That requires more code but I think it’s a bit more expressive and makes life easier for the user of the API.

An alternative approach which my colleague Lu Ning showed me and which I think is actually better is to make use of the object initializer syntax if all we have are setter methods on a builder.

We might therefore end up with something like this:

public class FooBuilder 
{
	public string Something = "DefaultSomething";
	public boolean SomethingElse = false;
 
	public Foo Build()
	{
		return new Foo(Something, SomethingElse);
	}
}

new FooBuilder { SomethingElse = true }.Build();

With this approach we end up writing less code and although we use public fields on the builder I don’t think it’s a big deal since it allows us to achieve our goal more quickly. If we need other methods that take out the complexity of construction then we can easily just add those as well.

Another thing to note when using this pattern is that we don’t need to override all the object’s attributes on every single test. We only need to override those ones which we are using in our test. The rest of the values can just be defaulted.

[Test]
public void ShouldDoSomething()
{
	var foo = new FooBuilder { Bar = "myBar", Baz = "myBaz", SomethingElse = "mySE", AndAgain = "..." }.Build();
 
 
	// and then further on we only check the value of Bar for example
 
	Assert.That(someValue, Is.EqualTo("myBar"); 
}

We don’t need to specifically set any of the values except ‘Bar’ because they are irrelevant and create a clutter in the test which means it takes longer to understand what’s going on.

This would be preferable:

[Test]
public void ShouldDoSomething()
{
	var foo = new FooBuilder { Bar = "myBar" }.Build();
 
 
	// and then further on we only check the value of Bar for example
 
	Assert.That(someValue, Is.EqualTo("myBar"); 
}

Something which I’ve been wondering about recently is understanding the best way to describe the case where we don’t want to define a specific value i.e. we want it as null.

I’d normally just set it to null like so:

new FooBuilder { Bar = null }.Build();

But we could make the API have a specific method and I haven’t really decided whether there’s much value to doing so yet:

new FooBuilder().WithNoBar().Build();

I was looking through some of the F# code I’ve written recently and I realised that I was very much writing C# in F# with respect to the number of if statements I’ve been using.

I thought it would be interesting to see what the code would look like if I was able to refactor some of that code to make use of pattern matching instead which would be a more idiomatic way of solving the problem in F#.

The first example of if statements is in my post about my F# solution to Roy Osherove’s TDD Kata.

I originally wrote a parse function which was able to parse a string and give it’s decimal value or 0 if it couldn’t be parsed.

let parse value = 
    let (itParsed, value) = Decimal.TryParse value
    if (itParsed) then value else 0.0m

If we use a pattern match expression we’d end up with the following:

let parse value = match Decimal.TryParse value with | (true, value) -> value | (false, _) -> 0.0m

The neat thing about this approach is that we don’t need to store the result of the ‘Decimal.TryParse’ function as we did in my original version.

We could in theory also write it like this…

let parse value = match Decimal.TryParse value with | (true, value) -> value | (_, _) -> 0.0m

…but while that is slightly less code I quite like the other version because it’s a bit more intention revealing that 0 will be returned if we fail to parse the string. I think there’s less thinking needed to understand the code.

Another example is this bit of code:

let add value = if ("".Equals(value) or "\n".Equals(value)) then 0.0m
                else match digits value |> Array.filter (fun x -> x < 1000m) with 
                     | ContainsNegatives(negatives) -> raise (ArgumentException (buildExceptionMessage negatives))
                     | NoNegatives(digits)          -> digits |> Array.sum

If we convert that to use pattern matching we would get this:

let add value = match value with
                | "" -> 0.0m
                | "\n" -> 0.0m
                | value ->  match digits value |> Array.filter (fun x -> x < 1000m) with 
                            | ContainsNegatives(negatives) -> raise (ArgumentException (buildExceptionMessage negatives))
                            | NoNegatives(digits)          -> digits |> Array.sum

That’s maybe slightly easier to read mainly because of the splitting up of the two inputs which lead to a 0 result.

The final example of using if statements is the following bit of code:

let (|CustomDelimeter|NoCustomDelimeter|) (value:string) = 
	...
 
     if (value.Length > 2 && "//".Equals(value.Substring(0, 2))) then
         if ("[".Equals(value.Substring(2,1))) then CustomDelimeter(delimeters value)
         else CustomDelimeter([| value.Substring(2, value.IndexOf("\n") - 2) |])
     else NoCustomDelimeter(",")

The only way I could see how to make this use active patterns is on the boolean statements like so:

let (|CustomDelimeter|NoCustomDelimeter|) (value:string) = 
	...
 
    match (value.Length > 2 && "//".Equals(value.Substring(0, 2))) with
    | true -> match ("[".Equals(value.Substring(2,1))) with 
              | true  ->  CustomDelimeter(delimeters value)
              | false ->  CustomDelimeter([| value.Substring(2, value.IndexOf("\n") - 2) |])
    | false -> NoCustomDelimeter(",")

I quite like that it lines up the return values of the active pattern which seems to make it a bit more readable.

I think overall maybe the pattern matching versions are slightly more readable but maybe not by much. What do you think?

I wrote previously about my dislike of C#’s object initializer syntax and while I still think those arguments hold I came across an interesting argument for why it is a useful feature in Jeremy Miller’s MSDN article on creating internal DSLs in C#.

In the article Jeremy works through an example where he builds up a ‘SendMessageRequest’ first by using a fluent interface and then by making use of object initializer syntax.

The fluent interface example reads like this:

public void SendMessageFluently(FluentMessageSender sender)
 {
     sender
         .SendText("the message body")
         .From("PARTNER001").To("PARTNER002");
 }

Compared with the object initializer version which looks like this:

public void SendMessageAsParameter(ParameterObjectMessageSender sender)
 {
     sender.Send(new SendMessageRequest()
     {
         Text = "the message body",
         Receiver = "PARTNER001",
         Sender = "PARTNER002"
     });
 }

As Jeremy points out:

this third incarnation of the API reduces errors in usage with much simpler mechanics than the fluent interface version.

You need much less code to get the second version to work which my colleague Lu Ning pointed out to me when we were discussing how to use the builder pattern on a project we worked on together.

In general terms the benefit of using object initializer is that we get more expressive code and the intent of that code is clearer than we would be able to achieve by constructing the object using its constructor.

The down side is that it creates a mentality of using setters and the initialisation of the object potentially leaves several fields with null values.

We then end up either getting unexpected null reference exceptions throughout the code or we code defensively and check for nulls whenever we try to access one of the properties. Seeing as one of the goals of the object initializer is to reduce the code we write it’s somewhat ironic that we end up writing this extra code to ensure the object has been setup correctly.

One way to solve this is to make use of tiny types so that the API is still expressive even though what we want to pass in is a string. That takes more code but it allows us to make use of the constructor to construct the object which is something which seems to have lost favour.

C# 4.0 will introduced named parameters into the language which seems like it will help us achieve expressiveness of our code without having to write extra classes and code to achieve this.

* Updated the examples as I had them the wrong way around. Thanks to Corey Haines and Mike Wagg for pointing it out.

As I’ve mentioned in a few of my recent posts I’ve been having another go at Roy Osherove’s TDD Kata but this time in F#.

One thing I’ve been struggling with when coding in F# is working out how many intermediate variables we actually need. They can be useful for expressing intent better but they’re clutter in a way.

I’ve included my solution at the end and in the active pattern which determines whether or not we have a custom delimeter defined in our input string I can’t decide whether or not to create a value to represent the expressions that determine that.

3
4
5
6
7
8
9
10

    let (|CustomDelimeter|NoCustomDelimeter|) (value:string) = 
        ...
        let hasACustomDelimeter = value.Length > 2 && "//".Equals(value.Substring(0, 2))
 
        if (hasACustomDelimeter) then
            if ("[".Equals(value.Substring(2,1))) then CustomDelimeter(delimeters value)
            else CustomDelimeter([| value.Substring(2, value.IndexOf("\n") - 2) |])
        else NoCustomDelimeter(",")

In a way it’s quite obvious that the expression on line 3 is what we’re using to determine if the input string has a custom delimeter because we state that on the next line.

    let (|CustomDelimeter|NoCustomDelimeter|) (value:string) = 
        ...
        if (value.Length > 2 && "//".Equals(value.Substring(0, 2))) then
            if ("[".Equals(value.Substring(2,1))) then CustomDelimeter(delimeters value)
            else CustomDelimeter([| value.Substring(2, value.IndexOf("\n") - 2) |])
        else NoCustomDelimeter(",")

I can’t decide which I prefer so any thoughts on that would be welcome.

I ran into a bit of trouble trying to make the following requirement work because my original parse function was hiding the fact that the code was failing on this step:

Delimiters can be of any length with the following format:  “//[delimiter]\n” for example: “//***\n1***2***3” should return 6

The parse function was originally defined to returns a zero value if it failed to parse the string which meant that the function which decomposed the string into a sequences of numbers could fail and we wouldn’t see an exception, just a failing test.

    let parse value = 
        let (itParsed, value) = Decimal.TryParse value
        if (itParsed) then value else 0.0m

Having the function defined like this simplified the code a bit because I didn’t need to deal with ignoring some characters at the beginning of the string when a custom delimeter was being specified.

One of the instructions for the exercise is to focus on writing tests for the valid inputs and not for invalid inputs which I initially struggled with. Usually if I was test driving code I would have written tests against invalid inputs to help me drive out the design.

Once I started focusing on just making the test past instead of finding a generic solution for the whole problem this became much easier and I didn’t need to test with the invalid inputs.

I wrote tests for the code in C# using NUnit so that I could run the tests from Resharper. I still haven’t found a good way to run automated tests from inside Visual Studio when they’re written in F# otherwise I’d have probably just done that.

All the tests I wrote were against the ‘add’ function but the way the code is written at the moment it would be possible to write tests against the other functions directly if I wanted to.

If I was working in C# perhaps some of those functions would be classes and I would write tests directly against those but I haven’t done that here and I’m not sure whether it is necessary. ‘digits’ is the only function where that would seem to add value.

This is the code I’ve got at the moment:

module FSharpCalculator
    open System
    open System.Text.RegularExpressions
 
    let split (delimeter:array<string>) (value:string) = value.Split (delimeter, StringSplitOptions.None)
    let toDecimal value = Decimal.Parse value
 
    let (|CustomDelimeter|NoCustomDelimeter|) (value:string) = 
        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
 
        if (value.Length > 2 && "//".Equals(value.Substring(0, 2))) then
            if ("[".Equals(value.Substring(2,1))) then CustomDelimeter(delimeters value)
            else CustomDelimeter([| value.Substring(2, value.IndexOf("\n") - 2) |])
        else NoCustomDelimeter(",")    
 
    let digits value = match value with 
                       | CustomDelimeter(delimeters)  -> value.Substring(value.IndexOf("\n")) |> split delimeters  |> Array.map toDecimal 
                       | NoCustomDelimeter(delimeter) -> value.Replace("\n", delimeter) |> split [|delimeter |] |> Array.map toDecimal
 
    let buildExceptionMessage negatives = 
        sprintf "No negative numbers allowed. You provided %s" (String.Join(",", negatives |> Array.map (fun x -> x.ToString())))
 
    let (|ContainsNegatives|NoNegatives|) digits =
        if (digits |> Array.exists (fun x -> x < 0.0m)) 
        then ContainsNegatives(digits |> Array.filter (fun x -> x < 0.0m))
        else NoNegatives(digits)
 
    let add value = if ("".Equals(value) or "\n".Equals(value)) then 0.0m
                    else match digits value |> Array.filter (fun x -> x < 1000m) with 
                         | ContainsNegatives(negatives) -> raise (ArgumentException (buildExceptionMessage negatives))
                         | NoNegatives(digits)          -> digits |> Array.sum