Skip to content

Commit

Permalink
fix negative variable evaluation on formula questions
Browse files Browse the repository at this point in the history
fixes CNVS-9970
If you use a negative variable as the first element in a formula
expression, formula question operation breaks.  For example, when you
put:

-[x]+5

in the body of the formula question,  and then attempt to put:

-x+5

in the formula definition, the formula question JavaScript does not know
how to parse the expression. This commit adds graceful logic for these cases.

Test plan
- Create a formula question with the initial variable cast as negative
- Formula definitions should be calculated correctly.
Regression
- Confirm that formula questions still work correctly with positive
  variables

Change-Id: I8d2c6f596bb0211e375c5a347dfa8a1ea2870271
Reviewed-on: https://gerrit.instructure.com/33003
Tested-by: Jenkins <[email protected]>
Reviewed-by: Stanley Stuart <[email protected]>
Reviewed-by: Jason Madsen <[email protected]>
QA-Review: Caleb Guanzon <[email protected]>
Product-Review: Josh Simpson <[email protected]>
  • Loading branch information
stderr committed Apr 9, 2014
1 parent 48a2cbd commit ef36875
Showing 1 changed file with 23 additions and 16 deletions.
39 changes: 23 additions & 16 deletions public/javascripts/calcCmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ define([
result = syntax[syntaxIndex];
break;
case 'subtract':
if(syntax[syntaxIndex + 1] && syntax[syntaxIndex + 1].token == 'number') {
if(syntax[syntaxIndex + 1] && (syntax[syntaxIndex + 1].token == 'number' || syntax[syntaxIndex + 1].token == 'variable')) {
syntax[syntaxIndex + 1].value = "-" + syntax[syntaxIndex + 1].value;
syntaxIndex++;
result = syntax[syntaxIndex];
Expand All @@ -75,8 +75,8 @@ define([
result.arguments = []
var ender = 'comma';
syntaxIndex = syntaxIndex + 2;
if(syntax[syntaxIndex].token == 'close_paren') {
ender = 'close_paren';
if(syntax[syntaxIndex].token == 'close_paren') {
ender = 'close_paren';
syntaxIndex++;
}
while(ender == 'comma') {
Expand Down Expand Up @@ -109,7 +109,7 @@ define([
};
var parseModifier = function(syntax) {
switch(syntax[syntaxIndex].token) {
case 'add':
case 'add':
return syntax[syntaxIndex++];
break;
case 'subtract':
Expand All @@ -129,7 +129,7 @@ define([
var index = (syntax && syntax[syntaxIndex] && syntax[syntaxIndex].newIndex) || 0;
throw("unexpected " + value + " at " + index);
};

var parseExpression = function(syntax, enders) {
var result = {
token: 'expression',
Expand Down Expand Up @@ -264,8 +264,15 @@ define([
if(tree.value == '_') {
return lastComputedResult || 0;
}
var value = predefinedVariables && predefinedVariables[tree.value];
value = value || (variables && variables[tree.value]);
if(tree.value.indexOf("-") == 0) { // the variable is negative, e.g. '-x'
var absolute = tree.value.replace(/^\-/, "")
var value = predefinedVariables && predefinedVariables[absolute];
value = value || (variables && variables[absolute]);
value = -value;
} else {
var value = predefinedVariables && predefinedVariables[tree.value];
value = value || (variables && variables[tree.value]);
}
if (value == undefined) {
throw("undefined variable " + tree.value);
}
Expand Down Expand Up @@ -369,10 +376,10 @@ define([
(function() {
var p = function(name, value, description) { calcCmd.addPredefinedVariable(name, value, description); }
var f = function(name, func, description, example) { calcCmd.addFunction(name, func, description, example); }

p('pi', Math.PI );
p('e', Math.exp(1));

f('abs', function(val) { return Math.abs(val) }, I18n.t('abs.description', "Returns the absolute value of the given value"), "abs(x)");
f('asin', function(x) { return Math.asin(x); }, I18n.t('asin.description', "Returns the arcsin of the given value"), "asin(x)");
f('acos', function(x) { return Math.acos(x); }, I18n.t('acos.description', "Returns the arccos of the given value"), "acos(x)");
Expand All @@ -393,7 +400,7 @@ define([
return args;
}
}
f('max', function() {
f('max', function() {
var args = make_list(arguments)
var max = args[0];
for(var idx = 0; idx < args.length; idx++) { //in arguments) {
Expand All @@ -410,15 +417,15 @@ define([
return min;
}, I18n.t('min.description', "Returns the lowest value in the list"), ["min(a,b,c...)", "min(list)"]);
f('sqrt', function(x) { return Math.sqrt(x); }, I18n.t('sqrt.description', "Returns the square root of the given value"), "sqrt(x)");
f('sort', function(x) {
f('sort', function(x) {
var args = make_list(arguments);
var list = [];
for(var idx = 0; idx < args.length; idx++) {
list.push(args[idx]);
}
return list.sort();
}, I18n.t('sort.description', "Returns the list of values, sorted from lowest to highest"), ["sort(a,b,c...)", "sort(list)"]);
f('reverse', function(x) {
f('reverse', function(x) {
var args = make_list(arguments);
var list = [];
for(var idx = 0; idx < args.length; idx++) {
Expand All @@ -427,9 +434,9 @@ define([
return list;
}, I18n.t('reverse.description', "Reverses the order of the list of values"), ["reverse(a,b,c...)", "reverse(list)"]);
f('first', function() { return make_list(arguments)[0]; }, I18n.t('first.description', "Returns the first value in the list"), ["first(a,b,c...)", "first(list)"]);
f('last', function() {
f('last', function() {
var args = make_list(arguments);
return args[args.length - 1];
return args[args.length - 1];
}, I18n.t('last.description', "Returns the last value in the list"), ["last(a,b,c...)", "last(list)"]);
f('at', function(list, x) { return list[x]; }, I18n.t('at.description', "Returns the indexed value in the given list"), "at(list,index)" );
f('rand', function(x) { return (Math.random() * (x || 1)); }, I18n.t('rand.description', "Returns a random number between zero and the range specified, or one if no number is given"), "rand(x)");
Expand All @@ -443,7 +450,7 @@ define([
}
return total;
}
f('mean', function() {
f('mean', function() {
var args = make_list(arguments);
return sum(args) / args.length;
}, I18n.t('mean.description', "Returns the average mean of the values in the list"), ["mean(a,b,c...)", "mean(list)"]);
Expand All @@ -460,7 +467,7 @@ define([
return ((list[Math.round(list.length / 2)] + list[Math.round(list.length / 2) - 1]) / 2);
}
}, I18n.t('median.description', "Returns the median for the list of values"), ["median(a,b,c...)", "median(list)"]);
f('range', function() {
f('range', function() {
var args = make_list(arguments);
var list = [];
for(var idx = 0; idx < args.length; idx++) {
Expand Down

0 comments on commit ef36875

Please sign in to comment.