Merge pull request #646 from isaacphysics/improvement/new-chemistry-o…

…ptions

Add allowScalingCoefficents option to Chemistry checker

src/main/java/uk/ac/cam/cl/dtg/isaac/dos/IsaacSymbolicChemistryQuestion.java

@@ -32,6 +32,7 @@ public class IsaacSymbolicChemistryQuestion extends IsaacSymbolicQuestion {
     @JsonProperty("isNuclear")
     private boolean isNuclear;
     private boolean allowPermutations;
+    private boolean allowScalingCoefficients;
 
     /**
      * @return whether the question is a nuclear question or not
@@ -58,4 +59,16 @@ public void setNuclear(boolean nuclear) {
     public void setAllowPermutations(boolean allowPermutations) {
         this.allowPermutations = allowPermutations;
     }
+
+    /**
+     * @return whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O
+     */
+    public boolean getAllowScalingCoefficients() { return allowScalingCoefficients; }
+
+    /**
+     * @param allowScalingCoefficients set whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O
+     */
+    public void setAllowScalingCoefficients(boolean allowScalingCoefficients) {
+        this.allowScalingCoefficients = allowScalingCoefficients;
+    }
 }

src/main/java/uk/ac/cam/cl/dtg/isaac/dto/IsaacSymbolicChemistryQuestionDTO.java

@@ -26,6 +26,7 @@ public class IsaacSymbolicChemistryQuestionDTO extends IsaacSymbolicQuestionDTO
     @JsonProperty("isNuclear")
     private boolean isNuclear;
     private boolean allowPermutations;
+    private boolean allowScalingCoefficients;
 
     /**
      * @return whether the question is a nuclear question or not
@@ -52,4 +53,16 @@ public void setNuclear(boolean nuclear) {
     public void setAllowPermutations(boolean allowPermutations) {
         this.allowPermutations = allowPermutations;
     }
+
+    /**
+     * @return whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O
+     */
+    public boolean getAllowScalingCoefficients() { return allowScalingCoefficients; }
+
+    /**
+     * @param allowScalingCoefficients set whether the question allows coefficients to be multiplied e.g. 10 H2 + 5 O2 -> 10 H2O
+     */
+    public void setAllowScalingCoefficients(boolean allowScalingCoefficients) {
+        this.allowScalingCoefficients = allowScalingCoefficients;
+    }
 }

src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacSymbolicChemistryValidator.java

@@ -30,6 +30,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 
 import static uk.ac.cam.cl.dtg.isaac.api.Constants.*;
 
@@ -55,6 +56,11 @@ private enum MatchType {
     private final String chemistryValidatorUrl;
     private final String nuclearValidatorUrl;
 
+    private final Set<String> VALID_ERROR_FEEDBACK = Set.of(
+            "Division by zero is undefined!",
+            "Your answer contains invalid syntax!"
+    );
+
     public IsaacSymbolicChemistryValidator(final String hostname, final String port) {
         this.nuclearValidatorUrl =  "http://" + hostname + ":" + port + "/nuclear/check";
         this.chemistryValidatorUrl = "http://" + hostname + ":" + port + "/chemistry/check";
@@ -192,6 +198,7 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi
                     req.put("test", submittedFormula.getMhchemExpression());
                     req.put("description", chemistryQuestion.getId());
                     req.put("allowPermutations", String.valueOf(chemistryQuestion.getAllowPermutations()));
+                    req.put("allowScalingCoefficients", String.valueOf(chemistryQuestion.getAllowScalingCoefficients()));
                     req.put("questionID", question.getId());
 
                     if (chemistryQuestion.isNuclear()) {
@@ -212,9 +219,9 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi
                                     + "\" with symbolic chemistry checker: " + response.get("error"));
                         }
 
+                        closestMatch = formulaChoice;
+                        closestResponse = response;
                         containsError = true;
-                        // If parsing was unsuccessful the student provided the wrong type
-                        isNuclear = !chemistryQuestion.isNuclear();
                         break;
                     }
 
@@ -324,9 +331,12 @@ public QuestionValidationResponse validateQuestionResponse(final Question questi
             if (containsError) {
 
                 // User input contains error terms.
-                // FIXME: This currently clashes with determining whether the submitted answer was the wrong type
-                // Inequality should be changed to not allow Nuclear syntax in Chemistry questions and vice versa
-                feedback = new Content("Your answer contains invalid syntax!");
+                if (closestResponse != null && VALID_ERROR_FEEDBACK.contains((String) closestResponse.get("error"))) {
+                    feedback = new Content((String) closestResponse.get("error"));
+                } else {
+                    // Default error message
+                    feedback = new Content("Your answer contains invalid syntax!");
+                }
 
             } else if (closestMatch != null && closestMatchType == MatchType.EXACT) {