Don't use student_formula to collect matrix array and update typeMatch methods.

[somiaj]

When collecting a matrix array answer use student_array instead of student_formula as student_formula should always be a formula, and this could fail to convert to a formula if an answer blank is left empty, causing a future typeMatch error.

Fixes #1406.

In addition update the typeMatch methods as suggested by @dpvc. When checking if $other is an object, check that it is actually a Value object instead just that it is a reference in the various typeMatch methods. This is done by using Value::isValue($other) instead of ref($other).

[dpvc]

The typeMatch() method was originally intended to be called only on MathObejcts, so I think it is important to determine why $other is not a MathObject in this case. It turns out that it is due to how ans_collect() and cmp_collect() handle the matrix array input data. They use $ans->{student_formula} to pass the array of formulas from ans_collect() to cmp_collect(). This is turned into an actual MathObejct Formula within cmp_collect(), but there is on case where that doesn't occur. When there are blank entries, ans_collec() returns false (0), and cmp_collect() returns before replacing student_formula. That means that student_formula is actually an array rathe than a MathObject Formula, as it should be.

It was probably a mistake to use student_formula for this, as that should always represent an actual Formula object. So my proposed correction would be to change ans_collect() to use a different property, say student_array to return the array to cmp_collect so that student_formula is always a MathObject, as it should be. Something like

diff --git a/lib/Value/AnswerChecker.pm b/lib/Value/AnswerChecker.pm
index ec6a6b9b..1d4eba90 100644
--- a/lib/Value/AnswerChecker.pm
+++ b/lib/Value/AnswerChecker.pm
@@ -201,9 +201,9 @@ sub cmp_collect {
        return 1 unless $self->{ans_name};
        $ans->{preview_latex_string} = $ans->{preview_text_string} = "";
        my $OK = $self->ans_collect($ans);
-       $ans->{student_ans} = $self->format_matrix($ans->{student_formula}, @{ $self->{format_options} }, tth_delims => 1);
+       $ans->{student_ans} = $self->format_matrix($ans->{student_array}, @{ $self->{format_options} }, tth_delims => 1);
        return 0 unless $OK;
-       my $array = $ans->{student_formula};
+       my $array = $ans->{student_array};
 
        if ($self->{ColumnVector}) {
                my @V = ();
@@ -735,7 +735,8 @@ sub ans_collect {
                }
                push(@array, [@row]);
        }
-       $ans->{student_formula} = [@array];
+       delete $ans->{student_formula};
+       $ans->{student_array} = [@array];
        $ans->{ans_message}     = $ans->{error_message} = "";
        if (scalar(@{$errors})) {
                $ans->{ans_message} = $ans->{error_message} =

This resolve the problem at the source, and potentially prevents future problems.

On the other hand, the Formula object's typeMatch() method probably does need work. All the other typeMatch() method include a test involving either ref($other) being non-empty, or Value::isValue($other) being true, so the Formula version should do that as well, and the fact that it doesn't is an oversight.

Note, however, that the other typeMatch() methods return 1 not 0 when $other isn't a Value object. This is because, a Real should be able to be compared to a perl real, for example, or a string (like 'DNE') without causing an error; the perl real will be promoted to a Real object, and a string will silently fail to equal it. Similarly, Formula('x+1')->typeMatch(3) should be true, since a formula can be compared to a perl real will be promoted to a constant Formula during comparison. So 0 is the wrong return value when $other isn't already an object.

I would recommend using Value::isValue($other) as the test, rather than just Value::isBlessed(), since it is Value objects that have the type() method. A number of the existing typeMatch() methods use ref($other), and those should probably be changed to Value::isValue() calls as well.

[somiaj]

I updated the fix as suggested by @dpvc to collect the matrix array into a new key, student_array instead of using student_formula which could not create a formula if an answer blank was missing.

I also added a second commit suggested by @dpvc to update the typeMatch methods to use Value::isValue for the check instead of ref and add the check to the Value::Formula::typeMatch method as well.