From eb9f224c609041dbd05510074e9e74b59e74bb88 Mon Sep 17 00:00:00 2001 From: Mike Cifelli Date: Sat, 24 Dec 2016 13:16:03 -0500 Subject: [PATCH] Refactored the COND built-in function --- src/function/ArgumentValidator.java | 32 +++++---- src/function/builtin/COND.java | 76 +++++++++++---------- test/function/ArgumentValidatorTester.java | 20 +++++- test/function/builtin/CONDTester.java | 77 ++++++++++++++++++++++ 4 files changed, 157 insertions(+), 48 deletions(-) create mode 100644 test/function/builtin/CONDTester.java diff --git a/src/function/ArgumentValidator.java b/src/function/ArgumentValidator.java index ba76318..aef0ebb 100644 --- a/src/function/ArgumentValidator.java +++ b/src/function/ArgumentValidator.java @@ -50,26 +50,25 @@ 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); - + validateListLength(argumentList); validateArgumentTypes(argumentList); } private void validateListNotDotted(Cons argumentList) { - Cons currentCons = argumentList; - SExpression nextCons = argumentList.getCdr(); + SExpression next = argumentList.getCdr(); - while (!nextCons.nullp()) { - if (!nextCons.consp()) - throw new DottedArgumentListException(functionName, argumentList); + for (Cons current = argumentList; next.consp(); next = current.getCdr()) + current = (Cons) next; - currentCons = (Cons) nextCons; - nextCons = currentCons.getCdr(); - } + if (!next.nullp()) + throw new DottedArgumentListException(functionName, argumentList); + } + + private void validateListLength(Cons argumentList) { + if (containsTooFewArguments(argumentList)) + throw new TooFewArgumentsException(functionName, argumentList); + else if (containsTooManyArguments(argumentList)) + throw new TooManyArgumentsException(functionName, argumentList); } private boolean containsTooFewArguments(Cons argumentList) { @@ -101,6 +100,11 @@ public class ArgumentValidator { return trailingArgumentType.isInstance(remainingArgument); } + public void validateListIsNotNil(Cons list) { + if (list.nullp()) + throw new BadArgumentTypeException(functionName, Nil.getUniqueInstance(), Cons.class); + } + public static class TooFewArgumentsException extends LispException { private static final long serialVersionUID = 1L; diff --git a/src/function/builtin/COND.java b/src/function/builtin/COND.java index 7f7da02..b7a4c00 100644 --- a/src/function/builtin/COND.java +++ b/src/function/builtin/COND.java @@ -1,53 +1,63 @@ package function.builtin; -import function.LispFunction; +import function.*; import sexpression.*; public class COND extends LispFunction { - public SExpression call(Cons argList) { - if (argList.nullp()) { - // return NIL if there are were no arguments passed to COND + private ArgumentValidator argumentValidator; + + public COND() { + this.argumentValidator = new ArgumentValidator("COND"); + this.argumentValidator.setEveryArgumentExpectedType(Cons.class); + } + + public SExpression call(Cons argumentList) { + argumentValidator.validate(argumentList); + + return callTailRecursive(argumentList); + } + + private SExpression callTailRecursive(Cons argumentList) { + if (argumentList.nullp()) return Nil.getUniqueInstance(); - } - SExpression argCar = argList.getCar(); // first clause - Cons argCdr = (Cons) argList.getCdr(); // list of remaining clauses + Cons clause = getFirstClauseAndValidateNotNil(argumentList); + Cons remainingClauses = (Cons) argumentList.getCdr(); + SExpression test = EVAL.eval(clause.getCar()); - // make sure the first clause is a list and is not NIL - if (argCar.consp()) { - Cons clause = (Cons) argCar; - SExpression test = EVAL.eval(clause.getCar()); + if (isTestSuccessful(test)) + return evaluateResult(clause, test); - if (test != Nil.getUniqueInstance()) { - // the car of this clause is true, so we evaluate its cdr + return callTailRecursive(remainingClauses); + } - SExpression cdr = clause.getCdr(); - SExpression retval = test; + private Cons getFirstClauseAndValidateNotNil(Cons argumentList) { + Cons firstClause = (Cons) argumentList.getCar(); + argumentValidator.validateListIsNotNil(firstClause); - // evaluate all the S-expressions in the cdr of the clause - while (cdr.consp()) { - retval = EVAL.eval(((Cons) cdr).getCar()); - cdr = ((Cons) cdr).getCdr(); - } + return firstClause; + } - // return the value of the last S-expression evaluated - return retval; - } + private boolean isTestSuccessful(SExpression test) { + return test != Nil.getUniqueInstance(); + } - // the car of this clause is false, so we test any remaining - // clauses + private SExpression evaluateResult(Cons clause, SExpression test) { + SExpression lastResultValue = test; - // check if the list of remaining clauses is a list and is not NIL - if (argCdr.consp()) { - return call(argCdr); - } + for (SExpression result = clause.getCdr(); result.consp(); result = advanceCons(result)) + lastResultValue = EVAL.eval(getCar(result)); - // there are no remaining clauses, so we return NIL - return Nil.getUniqueInstance(); - } + return lastResultValue; + } - throw new RuntimeException("COND: clause " + argCar + " should be a list"); + private SExpression advanceCons(SExpression knownCons) { + return ((Cons) knownCons).getCdr(); + } + + private SExpression getCar(SExpression knownCons) { + return ((Cons) knownCons).getCar(); } public boolean evaluateArguments() { diff --git a/test/function/ArgumentValidatorTester.java b/test/function/ArgumentValidatorTester.java index 8a14947..7d01618 100644 --- a/test/function/ArgumentValidatorTester.java +++ b/test/function/ArgumentValidatorTester.java @@ -165,7 +165,8 @@ public class ArgumentValidatorTester { try { validator.validate(argumentList); } catch (BadArgumentTypeException e) { - e.getMessage(); + assertNotNull(e.getMessage()); + assertTrue(e.getMessage().length() > 0); } } @@ -183,6 +184,13 @@ public class ArgumentValidatorTester { validator.validate(argumentList); } + @Test + public void DottedArgumentListException_HasCorrectSeverity() { + DottedArgumentListException e = new DottedArgumentListException("TEST", Nil.getUniqueInstance()); + + assertTrue(e.getSeverity() < ErrorManager.CRITICAL_LEVEL); + } + @Test public void dottedArgumentListException_HasMessageText() { DottedArgumentListException e = new DottedArgumentListException("TEST", Nil.getUniqueInstance()); @@ -191,4 +199,14 @@ public class ArgumentValidatorTester { assertTrue(e.getMessage().length() > 0); } + @Test(expected = BadArgumentTypeException.class) + public void validateListNotNil_ThrowsExceptionWithNilValue() { + validator.validateListIsNotNil(Nil.getUniqueInstance()); + } + + @Test + public void validateListNotNil_DoesNotThrowExceptionWithNonNilValue() { + validator.validateListIsNotNil(new Cons(Nil.getUniqueInstance(), Nil.getUniqueInstance())); + } + } diff --git a/test/function/builtin/CONDTester.java b/test/function/builtin/CONDTester.java new file mode 100644 index 0000000..d3034a3 --- /dev/null +++ b/test/function/builtin/CONDTester.java @@ -0,0 +1,77 @@ +package function.builtin; + +import static testutil.TestUtilities.*; + +import org.junit.Test; + +import function.ArgumentValidator.BadArgumentTypeException; + +public class CONDTester { + + @Test + public void testCondWithNoArguments() { + String input = "(cond)"; + + assertSExpressionsMatch(evaluateString(input), parseString("nil")); + } + + @Test + public void testCondWithTrue() { + String input = "(cond (T))"; + + assertSExpressionsMatch(evaluateString(input), parseString("T")); + } + + @Test + public void testCondWithSingleExpression() { + String input = "(cond (T \"true\"))"; + + assertSExpressionsMatch(evaluateString(input), parseString("\"true\"")); + } + + @Test + public void testCondWithMultipleExpressions() { + String input = "(cond ((= 1 2) 2) ((= 1 2) 2) ((= 1 1) 3))"; + + assertSExpressionsMatch(evaluateString(input), parseString("3")); + } + + @Test + public void testCondWithMultipleConditionsMatching_ReturnFirstOne() { + String input = "(cond ((= 1 1) 2) ((= 1 1) 3))"; + + assertSExpressionsMatch(evaluateString(input), parseString("2")); + } + + @Test + public void testCondWithMultipleConditionsMatching_OnlyEvaluatesFirstOne() { + String input = "(cond ((= 1 1) 2) ((= 1 1) x))"; + + assertSExpressionsMatch(evaluateString(input), parseString("2")); + } + + @Test + public void testCondWithMultipleResultValues_OnlyReturnsLast() { + String input = "(cond ((= 1 1) 2 3 4))"; + + assertSExpressionsMatch(evaluateString(input), parseString("4")); + } + + @Test + public void testCondWithNoConditionMatching_ReturnsNil() { + String input = "(cond ((= 1 2) T) ((= 1 3) T))"; + + assertSExpressionsMatch(evaluateString(input), parseString("nil")); + } + + @Test(expected = BadArgumentTypeException.class) + public void testCondWithNilArgument_ThrowsException() { + evaluateString("(cond ())"); + } + + @Test(expected = BadArgumentTypeException.class) + public void testCondWithNonListArgument_ThrowsException() { + evaluateString("(cond o)"); + } + +}