diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacCoordinateQuestion.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacCoordinateQuestion.java index 24870a926b..44112cef9e 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacCoordinateQuestion.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacCoordinateQuestion.java @@ -29,6 +29,7 @@ public class IsaacCoordinateQuestion extends IsaacQuestionBase { private String[] suffixes; private String buttonText; + private Boolean disregardSignificantFigures; private Integer significantFiguresMin; private Integer significantFiguresMax; @@ -56,6 +57,25 @@ public void setOrdered(final Boolean ordered) { this.ordered = ordered; } + /** + * Gets whether to disregard significant figures, i.e. allow exact answers only. + * + * @return true if significant figures should be disregarded, false or null otherwise. + */ + public Boolean getDisregardSignificantFigures() { + return disregardSignificantFigures; + } + + /** + * Sets whether to disregard significant figures, i.e. allow exact answers only. + * + * @param disregardSignificantFigures + * - whether to disregard significant figures + */ + public void setDisregardSignificantFigures(final Boolean disregardSignificantFigures) { + this.disregardSignificantFigures = disregardSignificantFigures; + } + /** * Gets the minimum allowed number of significant figures. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java index 69700aeec5..9803cb2e94 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidator.java @@ -46,6 +46,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question IsaacCoordinateQuestion coordinateQuestion = (IsaacCoordinateQuestion) question; CoordinateChoice submittedChoice = (CoordinateChoice) answer; + // Get significant figures to validate with + int sigFigsMin = requireNonNullElse(coordinateQuestion.getSignificantFiguresMin(), NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES); + int sigFigsMax = requireNonNullElse(coordinateQuestion.getSignificantFiguresMax(), NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES); + // STEP 0: Is it even possible to answer this question? if (null == coordinateQuestion.getChoices() || coordinateQuestion.getChoices().isEmpty()) { @@ -58,6 +62,14 @@ public final QuestionValidationResponse validateQuestionResponse(final Question feedback = new Content("This question cannot be answered correctly."); } + // Only worry about broken significant figure rules if we are going to use them (i.e. if "exact match" is false) + if (null == coordinateQuestion.getDisregardSignificantFigures() || !coordinateQuestion.getDisregardSignificantFigures()) { + if (sigFigsMin < 1 || sigFigsMax < 1 || sigFigsMax < sigFigsMin) { + log.error("Question has broken significant figure rules! " + question.getId() + " src: " + question.getCanonicalSourceFile()); + feedback = new Content("This question cannot be answered correctly."); + } + } + // STEP 1: Did they provide a valid answer? if (null == feedback && (null == submittedChoice.getItems() || submittedChoice.getItems().isEmpty())) { @@ -93,10 +105,6 @@ public final QuestionValidationResponse validateQuestionResponse(final Question feedback = new Content("You did not provide the correct number of coordinates."); } - // Get significant figures to validate with - int sigFigsMin = requireNonNullElse(coordinateQuestion.getSignificantFiguresMin(), NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES); - int sigFigsMax = requireNonNullElse(coordinateQuestion.getSignificantFiguresMax(), NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES); - // STEP 2: If they did, does their answer match a known answer? if (null == feedback) { @@ -152,10 +160,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question CoordinateItem choiceItem = choiceItems.get(coordIndex); CoordinateItem submittedItem = submittedItems.get(coordIndex); // Check that the submitted item matches the choice item - if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { + if (!coordinateItemsMatch(submittedItem, choiceItem, coordinateQuestion, false)) { allItemsMatch = false; // On mismatch, check if the items would match without excess significant figures - if (!coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { + if (!coordinateItemsMatch(submittedItem, choiceItem, coordinateQuestion, true)) { allItemsMatchWithoutSigFigs = false; // Exit early on mismatch: break; @@ -186,10 +194,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question boolean submittedItemInChoiceItem = false; boolean itemInChoiceWithoutSigFigs = false; for (CoordinateItem choiceItem : choiceItems) { - if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { + if (coordinateItemsMatch(submittedItem, choiceItem, coordinateQuestion, false)) { submittedItemInChoiceItem = true; break; - } else if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { + } else if (coordinateItemsMatch(submittedItem, choiceItem, coordinateQuestion, true)) { // On mismatch, check if the items would match without excess significant figures itemInChoiceWithoutSigFigs = true; } @@ -228,10 +236,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question boolean choiceItemInSubmittedItems = false; boolean itemInSubmittedWithoutSigFigs = false; for (CoordinateItem submittedItem : submittedItems) { - if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, false)) { + if (coordinateItemsMatch(submittedItem, choiceItem, coordinateQuestion, false)) { choiceItemInSubmittedItems = true; break; - } else if (coordinateItemsMatch(submittedItem, choiceItem, sigFigsMin, sigFigsMax, true)) { + } else if (coordinateItemsMatch(submittedItem, choiceItem, coordinateQuestion, true)) { // On mismatch, check if the items would match without excess significant figures itemInSubmittedWithoutSigFigs = true; } @@ -272,8 +280,8 @@ public final QuestionValidationResponse validateQuestionResponse(final Question feedback = coordinateQuestion.getDefaultFeedback(); } - // If there was no default feedback, check for too few significant figures - if (feedbackIsNullOrEmpty(feedback) && submittedItems.stream().anyMatch(i -> i.getCoordinates().stream() + // If incorrect & no other feedback, check for too few significant figures + if (!responseCorrect && feedbackIsNullOrEmpty(feedback) && submittedItems.stream().anyMatch(i -> i.getCoordinates().stream() .anyMatch(c -> ValidationUtils.tooFewSignificantFigures(c, sigFigsMin, log)))) { feedback = new Content(DEFAULT_VALIDATION_RESPONSE); feedback.setTags(new HashSet<>(ImmutableList.of("sig_figs", "sig_figs_too_few"))); @@ -283,7 +291,10 @@ public final QuestionValidationResponse validateQuestionResponse(final Question } private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final CoordinateItem choiceItem, - final int sigFigsMin, final int sigFigsMax, final boolean allowTooManySigFigs) { + final IsaacCoordinateQuestion coordinateQuestion, final boolean allowTooManySigFigs) { + + int sigFigsMin = requireNonNullElse(coordinateQuestion.getSignificantFiguresMin(), NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES); + int sigFigsMax = requireNonNullElse(coordinateQuestion.getSignificantFiguresMax(), NUMERIC_QUESTION_DEFAULT_SIGNIFICANT_FIGURES); if (submittedItem.getCoordinates().size() != choiceItem.getCoordinates().size()) { return false; @@ -293,6 +304,11 @@ private boolean coordinateItemsMatch(final CoordinateItem submittedItem, final C String submittedValue = submittedItem.getCoordinates().get(dimension); String choiceValue = choiceItem.getCoordinates().get(dimension); + if (null != coordinateQuestion.getDisregardSignificantFigures() + && coordinateQuestion.getDisregardSignificantFigures()) { + return ValidationUtils.numericValuesMatch(choiceValue, submittedValue, null, log); + } + if (allowTooManySigFigs) { // Check if the submission has more significant figures than the allowed maximum if (ValidationUtils.tooManySignificantFigures(submittedValue, sigFigsMax, log)) { diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java index 915c0850cb..3ae81212d1 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java @@ -234,6 +234,26 @@ public final void isaacCoordinateValidator_TestDefaultSigFigsMin() { assertTrue(response.isCorrect()); } + @Test + public final void isaacCoordinateValidator_TestDisregardSignificantFigures() { + someCoordinateQuestion.setDisregardSignificantFigures(true); + + // Exact match should be correct + CoordinateChoice c = new CoordinateChoice(); + c.setItems(List.of(item1, item2Again)); + + QuestionValidationResponse response = validator.validateQuestionResponse(someCoordinateQuestion, c); + + assertTrue(response.isCorrect()); + + // Extra trailing 0 should still be correct + c.setItems(List.of(item1ExtraSigFig, item2Again)); + + response = validator.validateQuestionResponse(someCoordinateQuestion, c); + + assertTrue(response.isCorrect()); + } + @Test public final void isaacCoordinateValidator_TestSubsetOfCorrectChoice() { someCoordinateQuestion.setOrdered(false);