Added unit tests and refactored the length built in function

src/function/builtin/cons/LENGTH.java

@@ -2,62 +2,49 @@ package function.builtin.cons;
 
 import java.math.BigInteger;
 
-import function.LispFunction;
+import function.*;
 import sexpression.*;
 
-/**
- * <code>LENGTH</code> represents the LENGTH function in Lisp.
- */
 public class LENGTH extends LispFunction {
 
-    /**
-     * Returns the length of the given list.
-     *
-     * @param list
-     *            the list to determine the length of
-     * @return the length of <code>list</code>
-     */
     public static int getLength(Cons list) {
         LENGTH lengthFunction = new LENGTH();
-        LispNumber length = lengthFunction.call(LIST.makeList(list));
+        LispNumber length = lengthFunction.callWithoutArgumentValidation(LIST.makeList(list));
 
         return length.getValue().intValue(); // TODO - return BigInteger when all built-ins use
                                              // ArgumentValidator
     }
 
-    public LispNumber call(Cons argList) {
+    private ArgumentValidator argumentValidator;
-        // make sure we have received at least one argument
-        if (argList.nullp()) {
-            Cons originalSExpr = new Cons(new Symbol("LENGTH"), argList);
 
-            throw new RuntimeException("too few arguments given to LENGTH: " + originalSExpr);
+    public LENGTH() {
+        this.argumentValidator = new ArgumentValidator("LENGTH");
+        this.argumentValidator.setExactNumberOfArguments(1);
+        this.argumentValidator.setEveryArgumentExpectedType(Cons.class);
     }
 
-        SExpression argCar = argList.getCar();
+    public LispNumber call(Cons argumentList) {
-        SExpression argCdr = argList.getCdr();
+        argumentValidator.validate(argumentList);
 
-        // make sure we have received only one argument
+        return callTailRecursive(BigInteger.ZERO, argumentList);
-        if (!argCdr.nullp()) {
-            Cons originalSExpr = new Cons(new Symbol("LENGTH"), argList);
-
-            throw new RuntimeException("too many arguments given to LENGTH: " + originalSExpr);
     }
 
-        // make sure that the argument is a list
+    private LispNumber callWithoutArgumentValidation(Cons argumentList) {
-        if (argCar.listp()) {
+        return callTailRecursive(BigInteger.ZERO, argumentList);
-            Cons arg = (Cons) argCar;
-
-            if (arg.nullp()) {
-                return new LispNumber(BigInteger.ZERO);
     }
 
-            Cons cdr = LIST.makeList(arg.getCdr());
+    private LispNumber callTailRecursive(BigInteger accumulatedLength, Cons argumentList) {
-            LispNumber cdrLength = call(cdr);
+        Cons list = (Cons) argumentList.getCar();
+        Cons restOfList = LIST.makeList(list.getCdr());
 
-            return new LispNumber(BigInteger.ONE.add(cdrLength.getValue()));
+        if (list.nullp())
+            return new LispNumber(accumulatedLength);
+
+        return callTailRecursive(increment(accumulatedLength), restOfList);
     }
 
-        throw new RuntimeException("LENGTH: a proper list must not end with " + argCar);
+    private BigInteger increment(BigInteger number) {
+        return number.add(BigInteger.ONE);
     }
 
 }

src/function/builtin/math/DIVIDE.java

@@ -17,7 +17,7 @@ public class DIVIDE extends LispFunction {
         this.mathFunction = new MathFunction(this::getReciprocal, this::divide);
     }
 
-    public SExpression call(Cons argumentList) {
+    public LispNumber call(Cons argumentList) {
         argumentValidator.validate(argumentList);
 
         return mathFunction.callTailRecursive(argumentList);

src/function/builtin/math/MINUS.java

@@ -17,7 +17,7 @@ public class MINUS extends LispFunction {
         this.mathFunction = new MathFunction(this::additiveInverse, this::subtract);
     }
 
-    public SExpression call(Cons argumentList) {
+    public LispNumber call(Cons argumentList) {
         argumentValidator.validate(argumentList);
 
         return mathFunction.callTailRecursive(argumentList);

src/function/builtin/math/MULTIPLY.java

@@ -14,7 +14,7 @@ public class MULTIPLY extends LispFunction {
         this.mathFunction = new MathFunction(number -> number, this::multiply);
     }
 
-    public SExpression call(Cons argumentList) {
+    public LispNumber call(Cons argumentList) {
         argumentValidator.validate(argumentList);
 
         return mathFunction.callTailRecursive(new Cons(LispNumber.ONE, argumentList));

src/function/builtin/math/MathFunction.java

@@ -15,7 +15,7 @@ class MathFunction {
         this.multipleValueOperation = multipleValueOperation;
     }
 
-    public SExpression callTailRecursive(Cons argumentList) {
+    public LispNumber callTailRecursive(Cons argumentList) {
         Cons remainingArguments = (Cons) argumentList.getCdr();
         SExpression firstArgument = argumentList.getCar();
         LispNumber number1 = (LispNumber) firstArgument;

src/function/builtin/math/PLUS.java

@@ -14,7 +14,7 @@ public class PLUS extends LispFunction {
         this.mathFunction = new MathFunction(number -> number, this::add);
     }
 
-    public SExpression call(Cons argumentList) {
+    public LispNumber call(Cons argumentList) {
         argumentValidator.validate(argumentList);
 
         return mathFunction.callTailRecursive(new Cons(LispNumber.ZERO, argumentList));

test/function/builtin/cons/LENGTHTester.java

@@ -0,0 +1,47 @@
+package function.builtin.cons;
+
+import static testutil.TestUtilities.*;
+
+import org.junit.Test;
+
+import function.ArgumentValidator.*;
+
+public class LENGTHTester {
+
+    @Test
+    public void testLengthOfNil() {
+        String input = "(length '())";
+
+        assertSExpressionsMatch(evaluateString(input), parseString("0"));
+    }
+
+    @Test
+    public void testLengthOfListOfOneElement() {
+        String input = "(length '(1))";
+
+        assertSExpressionsMatch(evaluateString(input), parseString("1"));
+    }
+
+    @Test
+    public void testLengthOfListOfManyElements() {
+        String input = "(length '(1 2 3 4 5))";
+
+        assertSExpressionsMatch(evaluateString(input), parseString("5"));
+    }
+
+    @Test(expected = BadArgumentTypeException.class)
+    public void testLengthWithNonList() {
+        evaluateString("(length 'x)");
+    }
+
+    @Test(expected = TooManyArgumentsException.class)
+    public void testLengthWithTooManyArguments() {
+        evaluateString("(length '(1 2) '(1 2))");
+    }
+
+    @Test(expected = TooFewArgumentsException.class)
+    public void testLengthWithTooFewArguments() {
+        evaluateString("(length)");
+    }
+
+}