FormulasQuestion / moodle-qtype_formulas

Formulas question type for Moodle
17 stars 29 forks source link

substr() #139

Closed jotroT closed 6 months ago

jotroT commented 6 months ago

Hallo Philipp,

I want to check if an answer given in a numeric answer box is not given in scientific notation nor using powers of 10 (like 2E4 or 2*10 4). In search for the E or the 10 I tried to add to my variables.php the function substring() at line #1603 as sstr() ('sstr' is as well in the initialize_function_list() as ):

//TEST substring case 'sstr': if (!($sz == 3 && $typestr == 's,n,n')) { break; } $N = intval($values[1]); $M = intval($values[2]); $value=substr($values[0], $N, $M); $this->replace_middle($vstack, $expression, $l, $r, 's', $value ); return true;

But I always get the error: Variable 'sstr' has not been defined. in substitute_vname_by_variables.

So here are my questions:

  1. What is wrong with my definition of sstr (I just created sth like the definition of fill())?

  2. Would it be helpful if substr() is included in the list of the formulas-functions? (Since the defining of the didn't work I couldn't check if it works at least for my purpose)

Best regards Joachim

PhilippImhof commented 6 months ago

Hello Joachim, and thanks for reaching out.

Adding functions to the legacy code is a true pain. Congratulations that you have gotten this far. I will gladly look into your code in detail (probably today) and tell you, why it is not working.

Please note, however, that I am completely rewriting that part (you may want to have a look at PR #62 to read more about it). The next major version will have more string functions, e.g. substr() and probably also functions for converting a string to upper / lower case. Also, it will be much easier to add new functions in the future. This is to say that I will not integrate your code in the regular version and you might have to add it again as soon as you update the plugin. (There will probably be one, maximum two versions before the next major.)

Best regards

PhilippImhof commented 6 months ago

You mentionedinitialize_function_list(), but I still think you somehow did not or not correctly register your function there, because if I leave that out, I get the same error, and if I add it back, the error goes away.

AFAICT the following patch works, at least for an easy test case:

--- variables.php   2024-01-16 09:36:39.000000000 +0100
+++ variables.sstr.php  2024-01-16 09:34:50.000000000 +0100
@@ -474,7 +474,7 @@
         );
         $this->func_special = array_flip(
           array('fill', 'len', 'pick', 'sort', 'sublist', 'inv', 'map', 'sum', 'concat', 'join', 'str', 'diff', 'poly', 'normcdf',
-          'modpow', 'binomialpdf', 'binomialcdf')
+          'modpow', 'binomialpdf', 'binomialcdf', 'sstr')
         );
         $this->func_all = array_merge($this->func_const, $this->func_unary, $this->func_binary, $this->func_special);
         $this->binary_op_map = array_flip(
@@ -1600,6 +1600,15 @@
                 }
                 $this->replace_middle($vstack, $expression, $l, $r, 'ln', $diff);
                 return true;
+            case 'sstr':
+                if (!($sz == 3 && $typestr == 's,n,n')) {
+                    break;
+                }
+                $n = intval($values[1]);
+                $m = intval($values[2]);
+                $value = substr($values[0], $n, $m);
+                $this->replace_middle($vstack, $expression, $l, $r, 's', $value);
+                return true;
             default:
                 return false;   // If no match, then the expression will be evaluated as a mathematical expression.
         }
@@ -2187,6 +2196,7 @@
                 case 'binomialpdf':
                 case 'binomialcdf':
                 case 'modpow':
+                case 'sstr':
                     if (strlen($regs[6]) != 0 || strlen($regs[5]) == 0) {
                         return get_string('functiontakesthreeargs', 'qtype_formulas', $regs[2]);
                     }

Obligatory disclaimer: Please proceed with caution.

PhilippImhof commented 6 months ago

I am closing this. If you have any further question, please do not hesitate to ask and reopen the issue.