Added function to the argument validator and refactored some code

src/function/ArgumentValidator.java

@@ -8,20 +8,31 @@ import sexpression.*;
 
 public class ArgumentValidator {
 
-    private Class<? extends SExpression> argumentType;
+    private Class<? extends SExpression> firstArgumentType;
+    private Class<? extends SExpression> trailingArgumentType;
     private String functionName;
     private Integer maximumNumberOfArguments;
     private Integer minimumNumberOfArguments;
 
     public ArgumentValidator(String functionName) {
-        this.argumentType = SExpression.class;
+        this.firstArgumentType = SExpression.class;
+        this.trailingArgumentType = SExpression.class;
         this.functionName = functionName;
         this.minimumNumberOfArguments = null;
         this.maximumNumberOfArguments = null;
     }
 
-    public void setArgumentType(Class<? extends SExpression> argumentType) {
+    public void setFirstArgumentExpectedType(Class<? extends SExpression> argumentType) {
-        this.argumentType = argumentType;
+        this.firstArgumentType = argumentType;
+    }
+
+    public void setTrailingArgumentExpectedType(Class<? extends SExpression> argumentType) {
+        this.trailingArgumentType = argumentType;
+    }
+
+    public void setEveryArgumentExpectedType(Class<? extends SExpression> argumentType) {
+        this.firstArgumentType = argumentType;
+        this.trailingArgumentType = argumentType;
     }
 
     public void setMaximumNumberOfArguments(int maximumNumberOfArguments) {
@@ -38,12 +49,27 @@ public class ArgumentValidator {
     }
 
     public void validate(Cons argumentList) {
+        validateListNotDotted(argumentList);
+
         if (containsTooFewArguments(argumentList))
             throw new TooFewArgumentsException(functionName, argumentList);
         else if (containsTooManyArguments(argumentList))
             throw new TooManyArgumentsException(functionName, argumentList);
-        else if (!isExpectedArgumentType(argumentList.getCar()))
+
-            throw new BadArgumentTypeException(functionName, argumentList.getCar());
+        validateArgumentTypes(argumentList);
+    }
+
+    private void validateListNotDotted(Cons argumentList) {
+        Cons currentCons = argumentList;
+        SExpression nextCons = argumentList.getCdr();
+
+        while (!nextCons.nullp()) {
+            if (!nextCons.consp())
+                throw new DottedArgumentListException(functionName, argumentList);
+
+            currentCons = (Cons) nextCons;
+            nextCons = currentCons.getCdr();
+        }
     }
 
     private boolean containsTooFewArguments(Cons argumentList) {
@@ -54,8 +80,25 @@ public class ArgumentValidator {
         return (maximumNumberOfArguments != null) && (LENGTH.getLength(argumentList) > maximumNumberOfArguments);
     }
 
-    private boolean isExpectedArgumentType(SExpression firstArgument) {
+    private void validateArgumentTypes(Cons argumentList) {
-        return argumentType.isInstance(firstArgument);
+        if (!isExpectedFirstArgumentType(argumentList.getCar()))
+            throw new BadArgumentTypeException(functionName, argumentList.getCar(), firstArgumentType);
+
+        validateRemainingArguments(argumentList);
+    }
+
+    private boolean isExpectedFirstArgumentType(SExpression firstArgument) {
+        return firstArgumentType.isInstance(firstArgument);
+    }
+
+    private void validateRemainingArguments(Cons cons) {
+        for (cons = (Cons) cons.getCdr(); !cons.nullp(); cons = (Cons) cons.getCdr())
+            if (!isExpectedRemainingArgumentType(cons.getCar()))
+                throw new BadArgumentTypeException(functionName, cons.getCar(), trailingArgumentType);
+    }
+
+    private boolean isExpectedRemainingArgumentType(SExpression remainingArgument) {
+        return trailingArgumentType.isInstance(remainingArgument);
     }
 
     public static class TooFewArgumentsException extends LispException {
@@ -92,20 +135,44 @@ public class ArgumentValidator {
         }
     }
 
+    public static class DottedArgumentListException extends LispException {
+
+        private static final long serialVersionUID = 1L;
+        private String functionName;
+        private Cons originalSExpression;
+
+        public DottedArgumentListException(String functionName, Cons argumentList) {
+            this.functionName = functionName;
+            this.originalSExpression = new Cons(new Symbol(this.functionName), argumentList);
+        }
+
+        @Override
+        public String getMessage() {
+            return MessageFormat.format("dotted argument list given to {0}: {1}", functionName, originalSExpression);
+        }
+    }
+
     public static class BadArgumentTypeException extends LispException {
 
         private static final long serialVersionUID = 1L;
         private String functionName;
         private String argument;
+        private Class<? extends SExpression> expected;
 
-        public BadArgumentTypeException(String functionName, SExpression argument) {
+        public BadArgumentTypeException(String functionName, SExpression argument,
+                                        Class<? extends SExpression> expected) {
             this.functionName = functionName;
             this.argument = argument.toString();
+            this.expected = expected;
         }
 
         @Override
         public String getMessage() {
-            return MessageFormat.format("{0}: {1} is not the expected type", functionName, argument);
+            DisplayName displayName = expected.getAnnotation(DisplayName.class);
+            String expectedType = (displayName == null) ? "unknown" : displayName.value();
+
+            return MessageFormat.format("{0}: {1} is not the expected type of ''{2}''", functionName, argument,
+                                        expectedType);
         }
     }
 

src/function/UserDefinedFunction.java

@@ -8,32 +8,20 @@ import table.SymbolTable;
 
 public class UserDefinedFunction extends LispFunction {
 
-    private ArgumentValidator argumentValidator;
     private String name;
     private Cons body;
     private Cons lambdaExpression;
     private SymbolTable environment;
     private ArrayList<String> formalParameters;
+    private ArgumentValidator argumentValidator;
 
-    /**
-     * Create a new user-defined function with the specified name, lambda list and body.
-     *
-     * @param name
-     *            the name of this user-defined function
-     * @param lambdaList
-     *            a list of the formal parameters of this user-defined function (MUST BE A PROPER
-     *            LIST)
-     * @param body
-     *            the body of this user-defined function (MUST BE A PROPER LIST)
-     */
     public UserDefinedFunction(String name, Cons lambdaList, Cons body) {
         this.name = name;
         this.body = body;
         this.lambdaExpression = new Cons(new Symbol(name), new Cons(lambdaList, body));
         this.environment = SETF.getEnvironment();
-        this.formalParameters = new ArrayList<String>();
 
-        for (; lambdaList.consp(); lambdaList = (Cons) lambdaList.getCdr())
+        for (this.formalParameters = new ArrayList<>(); lambdaList.consp(); lambdaList = (Cons) lambdaList.getCdr())
             this.formalParameters.add(lambdaList.getCar().toString());
 
         this.argumentValidator = new ArgumentValidator(this.name);

src/function/builtin/APPLY.java

@@ -5,7 +5,6 @@ import sexpression.*;
 
 public class APPLY extends LispFunction {
 
-    private static final int NUMBER_OF_ARGUMENTS = 2;
     private ArgumentValidator argumentValidator;
 
     public static SExpression apply(Cons argList) {
@@ -14,7 +13,8 @@ public class APPLY extends LispFunction {
 
     public APPLY() {
         this.argumentValidator = new ArgumentValidator("APPLY");
-        this.argumentValidator.setExactNumberOfArguments(NUMBER_OF_ARGUMENTS);
+        this.argumentValidator.setExactNumberOfArguments(2);
+        this.argumentValidator.setTrailingArgumentExpectedType(Cons.class);
     }
 
     public SExpression call(Cons argList) {
@@ -23,26 +23,9 @@ public class APPLY extends LispFunction {
         Cons cdr = (Cons) argList.getCdr();
         SExpression functionName = argList.getCar();
         SExpression argumentList = cdr.getCar();
+        LispFunction function = EVAL.lookupFunctionOrLambda(functionName);
 
-        if (argumentList.listp()) {
+        return function.call((Cons) argumentList);
-            LispFunction function = EVAL.lookupFunction(functionName.toString());
-
-            if (function == null) {
-                if (functionName.functionp()) {
-                    function = ((LambdaExpression) functionName).getFunction();
-                } else if (LAMBDA.isLambdaExpression(functionName)) {
-                    Cons lexpr = (Cons) functionName;
-
-                    function = LAMBDA.createFunction(lexpr);
-                } else {
-                    throw new RuntimeException("undefined function " + functionName);
-                }
-            }
-
-            return function.call((Cons) argumentList);
-        }
-
-        throw new RuntimeException("APPLY: " + argumentList + " is not a list");
     }
 
 }

src/function/builtin/ATOM.java

@@ -5,12 +5,11 @@ import sexpression.*;
 
 public class ATOM extends LispFunction {
 
-    private static final int NUMBER_OF_ARGUMENTS = 1;
     private ArgumentValidator argumentValidator;
 
     public ATOM() {
         this.argumentValidator = new ArgumentValidator("ATOM");
-        this.argumentValidator.setExactNumberOfArguments(NUMBER_OF_ARGUMENTS);
+        this.argumentValidator.setExactNumberOfArguments(1);
     }
 
     public SExpression call(Cons argumentList) {

src/function/builtin/EVAL.java

@@ -2,19 +2,14 @@ package function.builtin;
 
 import java.util.HashMap;
 
-import function.*;
+import function.LispFunction;
 import sexpression.*;
 
-/**
- * <code>EVAL</code> represents the EVAL function in Lisp.
- */
 public class EVAL extends LispFunction {
 
-    // A table to contain all the built-in and user-defined Lisp functions.
     private static HashMap<String, LispFunction> functionTable = new HashMap<String, LispFunction>();
 
     static {
-        // place all of the built-in functions into the function table
         functionTable.put("*", new MULTIPLY());
         functionTable.put("+", new PLUS());
         functionTable.put("-", new MINUS());
@@ -50,33 +45,32 @@ public class EVAL extends LispFunction {
         functionTable.put("SYMBOL-FUNCTION", new SYMBOL_FUNCTION());
     }
 
-    /**
-     * Retrieve the function table.
-     *
-     * @return the function table
-     */
     public static HashMap<String, LispFunction> getFunctionTable() {
         return functionTable;
     }
 
-    /**
-     * Look up a function by its name.
-     *
-     * @param functionName
-     *            the name of the function to look up
-     * @return the function with the name <code>functionName</code> if it exists; null otherwise
-     */
     public static LispFunction lookupFunction(String functionName) {
         return functionTable.get(functionName);
     }
 
-    /**
+    public static LispFunction lookupFunctionOrLambda(SExpression functionExpression) {
-     * Look up a symbol's value using its name.
+        LispFunction function = lookupFunction(functionExpression.toString());
-     *
+
-     * @param symbolName
+        if (function == null)
-     *            the name of the symbol to look up (must not be null)
+            function = createLambdaFunction(functionExpression);
-     * @return the value of <code>symbolName</code> if it has one; null otherwise
+
-     */
+        return function;
+    }
+
+    private static LispFunction createLambdaFunction(SExpression lambdaExpression) {
+        if (lambdaExpression.functionp())
+            return ((LambdaExpression) lambdaExpression).getFunction();
+        else if (LAMBDA.isLambdaExpression(lambdaExpression))
+            return LAMBDA.createFunction((Cons) lambdaExpression);
+        else
+            throw new RuntimeException("undefined function " + lambdaExpression);
+    }
+
     public static SExpression lookupSymbol(String symbolName) {
         if (symbolName.equals("NIL")) {
             return Nil.getUniqueInstance();
@@ -89,13 +83,6 @@ public class EVAL extends LispFunction {
         return SETF.lookup(symbolName);
     }
 
-    /**
-     * Determine if the given list is dotted.
-     *
-     * @param list
-     *            the list to be tested (must not be null)
-     * @return <code>true</code> if <code>list</code> is dotted; <code>false</code> otherwise
-     */
     public static boolean isDotted(Cons list) {
         if (list.nullp()) {
             return false;
@@ -111,13 +98,6 @@ public class EVAL extends LispFunction {
         return true;
     }
 
-    /**
-     * Evaluate the given S-expression.
-     *
-     * @param sexpr
-     *            the S-expression to evaluate
-     * @return the value of <code>sexpr</code>
-     */
     public static SExpression eval(SExpression sexpr) {
         Cons expList = LIST.makeList(sexpr);
         EVAL evalFunction = new EVAL();
@@ -125,7 +105,6 @@ public class EVAL extends LispFunction {
         return evalFunction.call(expList);
     }
 
-    // The number of arguments that EVAL takes.
     private static final int NUM_ARGS = 1;
 
     public SExpression call(Cons argList) {
@@ -164,29 +143,11 @@ public class EVAL extends LispFunction {
         return arg; // 'arg' is a NUMBER or a STRING
     }
 
-    // Evaluate the specified list.
-    //
-    // Parameters: list - the list to evaluate
-    // Returns: the value of 'list'
-    // Precondition: 'list' must not be null.
     private SExpression evaluateList(Cons list) {
         SExpression car = list.getCar();
         SExpression cdr = list.getCdr();
 
-        LispFunction function = lookupFunction(car.toString());
+        LispFunction function = lookupFunctionOrLambda(car);
-
-        if (function == null) {
-            // check if the car of the list is a lambda expression
-            if (car.functionp()) {
-                function = ((LambdaExpression) car).getFunction();
-            } else if (LAMBDA.isLambdaExpression(car)) {
-                Cons lexpr = (Cons) car;
-
-                function = LAMBDA.createFunction(lexpr);
-            } else {
-                throw new RuntimeException("undefined function " + car);
-            }
-        }
 
         // make sure the list of arguments for 'function' is a list
         if (cdr.listp()) {
@@ -210,12 +171,6 @@ public class EVAL extends LispFunction {
         throw new RuntimeException("argument list given to " + car + " is dotted: " + list);
     }
 
-    // Evaluate a list of arguments for a function.
-    //
-    // Parameters: arguments - a list of arguments for a function
-    // Returns: a list consisting of the values of the S-expressions found in
-    // 'arguments'
-    // Precondition: 'arguments' must not be null.
     private Cons evaluateArgList(Cons arguments) {
         if (arguments.nullp()) {
             return Nil.getUniqueInstance();

src/function/builtin/PLUS.java

@@ -3,9 +3,6 @@ package function.builtin;
 import function.LispFunction;
 import sexpression.*;
 
-/**
- * <code>PLUS</code> represents the '+' function in Lisp.
- */
 public class PLUS extends LispFunction {
 
     public LispNumber call(Cons argList) {
@@ -13,6 +10,9 @@ public class PLUS extends LispFunction {
             return new LispNumber(0);
         }
 
+        if (!argList.getCdr().listp())
+            throw new RuntimeException("+: " + argList + " is dotted");
+
         SExpression argFirst = argList.getCar();
         Cons argRest = (Cons) argList.getCdr();
 

src/sexpression/Atom.java

@@ -1,5 +1,6 @@
 package sexpression;
 
+@DisplayName("atom")
 public abstract class Atom extends SExpression {
 
     private String text;

src/sexpression/Cons.java

@@ -1,5 +1,6 @@
 package sexpression;
 
+@DisplayName("list")
 public class Cons extends SExpression {
 
     private SExpression car;

src/sexpression/DisplayName.java

@@ -0,0 +1,11 @@
+package sexpression;
+
+import java.lang.annotation.*;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface DisplayName {
+
+    String value();
+
+}

src/sexpression/LambdaExpression.java

@@ -2,6 +2,7 @@ package sexpression;
 
 import function.UserDefinedFunction;
 
+@DisplayName("lambda-expression")
 public class LambdaExpression extends SExpression {
 
     private Cons lambdaExpression;

src/sexpression/LispNumber.java

@@ -4,6 +4,7 @@ import java.text.MessageFormat;
 
 import error.LispException;
 
+@DisplayName("number")
 public class LispNumber extends Atom {
 
     private int value;

src/sexpression/LispString.java

@@ -1,5 +1,6 @@
 package sexpression;
 
+@DisplayName("string")
 public class LispString extends Atom {
 
     public LispString(String text) {

src/sexpression/Nil.java

@@ -1,5 +1,6 @@
 package sexpression;
 
+@DisplayName("nil")
 public class Nil extends Cons {
 
     private static Nil uniqueInstance = new Nil();

src/sexpression/SExpression.java

@@ -1,5 +1,6 @@
 package sexpression;
 
+@DisplayName("s-expression")
 public abstract class SExpression {
 
     public boolean nullp() {

src/sexpression/Symbol.java

@@ -1,5 +1,6 @@
 package sexpression;
 
+@DisplayName("symbol")
 public class Symbol extends Atom {
 
     public static final Symbol T = new Symbol("T");

test/function/ArgumentValidatorTester.java

@@ -103,14 +103,14 @@ public class ArgumentValidatorTester {
 
     @Test
     public void BadArgumentTypeException_HasCorrectSeverity() {
-        BadArgumentTypeException e = new BadArgumentTypeException("TEST", Nil.getUniqueInstance());
+        BadArgumentTypeException e = new BadArgumentTypeException("TEST", Nil.getUniqueInstance(), SExpression.class);
 
         assertTrue(e.getSeverity() < ErrorManager.CRITICAL_LEVEL);
     }
 
     @Test
     public void BadArgumentTypeException_HasMessageText() {
-        BadArgumentTypeException e = new BadArgumentTypeException("TEST", Nil.getUniqueInstance());
+        BadArgumentTypeException e = new BadArgumentTypeException("TEST", Nil.getUniqueInstance(), SExpression.class);
 
         assertNotNull(e.getMessage());
         assertTrue(e.getMessage().length() > 0);
@@ -118,14 +118,70 @@ public class ArgumentValidatorTester {
 
     @Test
     public void correctArgumentType_DoesNotThrowException() {
-        validator.setArgumentType(Nil.class);
+        validator.setEveryArgumentExpectedType(Nil.class);
         validator.validate(makeArgumentListOfSize(1));
     }
 
     @Test(expected = BadArgumentTypeException.class)
     public void badArgumentType_ThrowsException() {
-        validator.setArgumentType(LispString.class);
+        validator.setEveryArgumentExpectedType(LispString.class);
         validator.validate(makeArgumentListOfSize(1));
     }
 
+    @Test
+    public void correctFirstAndRestArgumentTypes_DoesNotThrowException() {
+        Cons argumentList = new Cons(Symbol.T, new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance()));
+
+        validator.setFirstArgumentExpectedType(Symbol.class);
+        validator.setTrailingArgumentExpectedType(Cons.class);
+        validator.validate(argumentList);
+    }
+
+    @Test(expected = BadArgumentTypeException.class)
+    public void badFirstArgumentType_ThrowsException() {
+        Cons argumentList = new Cons(Symbol.T, new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance()));
+
+        validator.setFirstArgumentExpectedType(Cons.class);
+        validator.setTrailingArgumentExpectedType(Cons.class);
+        validator.validate(argumentList);
+    }
+
+    @Test(expected = BadArgumentTypeException.class)
+    public void badTrailingArgumentType_ThrowsException() {
+        Cons argumentList = new Cons(Symbol.T, new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance()));
+
+        validator.setFirstArgumentExpectedType(Symbol.class);
+        validator.setTrailingArgumentExpectedType(Symbol.class);
+        validator.validate(argumentList);
+    }
+
+    @Test
+    public void expectedTypeWithNoDisplayName_DoesNotCauseNPE() {
+        Cons argumentList = new Cons(Symbol.T, new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance()));
+        SExpression withoutDisplayName = new SExpression() {};
+
+        validator.setEveryArgumentExpectedType(withoutDisplayName.getClass());
+
+        try {
+            validator.validate(argumentList);
+        } catch (BadArgumentTypeException e) {
+            e.getMessage();
+        }
+    }
+
+    @Test(expected = DottedArgumentListException.class)
+    public void givenDottedArgumentList_ThrowsException() {
+        Cons argumentList = new Cons(Symbol.T, Symbol.T);
+
+        validator.validate(argumentList);
+    }
+
+    @Test
+    public void dottedArgumentListException_HasMessageText() {
+        DottedArgumentListException e = new DottedArgumentListException("TEST", Nil.getUniqueInstance());
+
+        assertNotNull(e.getMessage());
+        assertTrue(e.getMessage().length() > 0);
+    }
+
 }

test/function/builtin/APPLYTester.java

@@ -43,7 +43,7 @@ public class APPLYTester {
         evaluateString("(apply 'f '(1 2 3))");
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test(expected = BadArgumentTypeException.class)
     public void testApplyWithNonListSecondArgument() {
         evaluateString("(apply '+ '2)");
     }