From 37bc303fe8bc54e8c4108b3357e1e2752e9e5ee0 Mon Sep 17 00:00:00 2001 From: Mike Cifelli Date: Sun, 25 Dec 2016 12:49:18 -0500 Subject: [PATCH] Improved the argument list validation for COND --- src/function/ArgumentValidator.java | 40 ++++++++++++++-------- src/function/builtin/COND.java | 10 ++---- test/function/ArgumentValidatorTester.java | 16 ++++++--- test/function/builtin/APPLYTester.java | 5 +++ test/function/builtin/CONDTester.java | 7 +++- 5 files changed, 51 insertions(+), 27 deletions(-) diff --git a/src/function/ArgumentValidator.java b/src/function/ArgumentValidator.java index aef0ebb..115b3c2 100644 --- a/src/function/ArgumentValidator.java +++ b/src/function/ArgumentValidator.java @@ -13,6 +13,7 @@ public class ArgumentValidator { private String functionName; private Integer maximumNumberOfArguments; private Integer minimumNumberOfArguments; + private boolean isNilAcceptable; public ArgumentValidator(String functionName) { this.firstArgumentType = SExpression.class; @@ -20,6 +21,7 @@ public class ArgumentValidator { this.functionName = functionName; this.minimumNumberOfArguments = null; this.maximumNumberOfArguments = null; + this.isNilAcceptable = true; } public void setFirstArgumentExpectedType(Class argumentType) { @@ -48,6 +50,10 @@ public class ArgumentValidator { this.maximumNumberOfArguments = exactNumberOfArguments; } + public void doNotAcceptNil() { + this.isNilAcceptable = false; + } + public void validate(Cons argumentList) { validateListNotDotted(argumentList); validateListLength(argumentList); @@ -80,29 +86,35 @@ public class ArgumentValidator { } private void validateArgumentTypes(Cons argumentList) { - if (!isExpectedFirstArgumentType(argumentList.getCar())) - throw new BadArgumentTypeException(functionName, argumentList.getCar(), firstArgumentType); + validateFirstArgument(argumentList); + validateTrailingArguments(argumentList); + } - validateRemainingArguments(argumentList); + private void validateFirstArgument(Cons argumentList) { + if (!isFirstArgumentValid(argumentList)) + throw new BadArgumentTypeException(functionName, argumentList.getCar(), firstArgumentType); + } + + private boolean isFirstArgumentValid(Cons argumentList) { + return argumentList.nullp() || isExpectedFirstArgumentType(argumentList.getCar()); } private boolean isExpectedFirstArgumentType(SExpression firstArgument) { - return firstArgumentType.isInstance(firstArgument); + return firstArgumentType.isInstance(firstArgument) && !isDisallowedNil(firstArgument); } - private void validateRemainingArguments(Cons cons) { - for (cons = (Cons) cons.getCdr(); !cons.nullp(); cons = (Cons) cons.getCdr()) - if (!isExpectedRemainingArgumentType(cons.getCar())) + private boolean isDisallowedNil(SExpression argument) { + return !isNilAcceptable && argument.nullp(); + } + + private void validateTrailingArguments(Cons argumentList) { + for (Cons cons = (Cons) argumentList.getCdr(); !cons.nullp(); cons = (Cons) cons.getCdr()) + if (!isExpectedTrailingArgumentType(cons.getCar())) throw new BadArgumentTypeException(functionName, cons.getCar(), trailingArgumentType); } - private boolean isExpectedRemainingArgumentType(SExpression remainingArgument) { - return trailingArgumentType.isInstance(remainingArgument); - } - - public void validateListIsNotNil(Cons list) { - if (list.nullp()) - throw new BadArgumentTypeException(functionName, Nil.getUniqueInstance(), Cons.class); + private boolean isExpectedTrailingArgumentType(SExpression trailingArgument) { + return trailingArgumentType.isInstance(trailingArgument) && !isDisallowedNil(trailingArgument); } public static class TooFewArgumentsException extends LispException { diff --git a/src/function/builtin/COND.java b/src/function/builtin/COND.java index b7a4c00..3171081 100644 --- a/src/function/builtin/COND.java +++ b/src/function/builtin/COND.java @@ -10,6 +10,7 @@ public class COND extends LispFunction { public COND() { this.argumentValidator = new ArgumentValidator("COND"); this.argumentValidator.setEveryArgumentExpectedType(Cons.class); + this.argumentValidator.doNotAcceptNil(); } public SExpression call(Cons argumentList) { @@ -22,7 +23,7 @@ public class COND extends LispFunction { if (argumentList.nullp()) return Nil.getUniqueInstance(); - Cons clause = getFirstClauseAndValidateNotNil(argumentList); + Cons clause = (Cons) argumentList.getCar(); Cons remainingClauses = (Cons) argumentList.getCdr(); SExpression test = EVAL.eval(clause.getCar()); @@ -32,13 +33,6 @@ public class COND extends LispFunction { return callTailRecursive(remainingClauses); } - private Cons getFirstClauseAndValidateNotNil(Cons argumentList) { - Cons firstClause = (Cons) argumentList.getCar(); - argumentValidator.validateListIsNotNil(firstClause); - - return firstClause; - } - private boolean isTestSuccessful(SExpression test) { return test != Nil.getUniqueInstance(); } diff --git a/test/function/ArgumentValidatorTester.java b/test/function/ArgumentValidatorTester.java index 7d01618..c102956 100644 --- a/test/function/ArgumentValidatorTester.java +++ b/test/function/ArgumentValidatorTester.java @@ -200,13 +200,21 @@ public class ArgumentValidatorTester { } @Test(expected = BadArgumentTypeException.class) - public void validateListNotNil_ThrowsExceptionWithNilValue() { - validator.validateListIsNotNil(Nil.getUniqueInstance()); + public void doNotAcceptNil_ThrowsExceptionOnNilArgument() { + validator.doNotAcceptNil(); + validator.validate(new Cons(Symbol.T, new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance()))); } @Test - public void validateListNotNil_DoesNotThrowExceptionWithNonNilValue() { - validator.validateListIsNotNil(new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance())); + public void doNotAcceptNil_AllowsEmptyArgumentList() { + validator.doNotAcceptNil(); + validator.validate(Nil.getUniqueInstance()); + } + + @Test + public void doNotAcceptNil_AllowsProperList() { + validator.doNotAcceptNil(); + validator.validate(new Cons(Symbol.T, new Cons(Symbol.T, Nil.getUniqueInstance()))); } } diff --git a/test/function/builtin/APPLYTester.java b/test/function/builtin/APPLYTester.java index 9b1b62d..70338c3 100644 --- a/test/function/builtin/APPLYTester.java +++ b/test/function/builtin/APPLYTester.java @@ -58,4 +58,9 @@ public class APPLYTester { evaluateString("(apply '1)"); } + @Test(expected = DottedArgumentListException.class) + public void testCondWithDottedArgumentList_ThrowsException() { + evaluateString("(apply 'apply (cons 'T 'T))"); + } + } diff --git a/test/function/builtin/CONDTester.java b/test/function/builtin/CONDTester.java index d3034a3..a6a1697 100644 --- a/test/function/builtin/CONDTester.java +++ b/test/function/builtin/CONDTester.java @@ -4,7 +4,7 @@ import static testutil.TestUtilities.*; import org.junit.Test; -import function.ArgumentValidator.BadArgumentTypeException; +import function.ArgumentValidator.*; public class CONDTester { @@ -74,4 +74,9 @@ public class CONDTester { evaluateString("(cond o)"); } + @Test(expected = DottedArgumentListException.class) + public void testCondWithDottedArgumentList_ThrowsException() { + evaluateString("(apply 'cond (cons '(nil T) 'b))"); + } + }