The math expression engine has several faults:

1) max() and min() don't work because functions can have only one argument.
2) There is no support for strings.
3) There is no support for if() or equality operators.

This significant retool improves this engine quite a bit, adding a simple if(), basic equality operators, fixing max() and min(), and adding support for strings as long as they're quoted. It also improves code style a little bit, though maybe not vastly, because I just used PHPStorm's re-format.

Patch forthcoming.

Files: 
CommentFileSizeAuthor
#26 drupal-1958538-8.patch46.38 KBrobertwb
PASSED: [[SimpleTest]]: [MySQL] 128 pass(es).
[ View ]
#26 interdiff.txt1.75 KBrobertwb
#23 interdiff.txt1.55 KBrobertwb
#21 drupal-1958538-7.patch46.52 KBrobertwb
PASSED: [[SimpleTest]]: [MySQL] 128 pass(es).
[ View ]
#19 drupal-1958538-6.patch46.52 KBrobertwb
FAILED: [[SimpleTest]]: [MySQL] 127 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#17 drupal-1958538-5.patch46.43 KBrobertwb
FAILED: [[SimpleTest]]: [MySQL] 127 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#15 drupal-1958538-4.patch35.79 KBrobertwb
FAILED: [[SimpleTest]]: [MySQL] 120 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#14 drupal-1958538-4.patch35.79 KBrobertwb
FAILED: [[SimpleTest]]: [MySQL] 120 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#10 drupal-1958538-3.patch35.72 KBrobertwb
FAILED: [[SimpleTest]]: [MySQL] 86 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#4 drupal-1958538-2.patch46.33 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 127 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#4 interdiff.txt10.7 KBdawehner
#2 drupal-1938062-66.patch24.16 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1938062-66_1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 interdiff.txt428 bytesdawehner
#1 1958538-math-expr-update.patch35.63 KBmerlinofchaos
PASSED: [[SimpleTest]]: [MySQL] 68 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new35.63 KB
PASSED: [[SimpleTest]]: [MySQL] 68 pass(es).
[ View ]

Notes:

This needs to be documented. I removed the docblock and old license because the rewrite is significant enough as to no longer require the old license, I believe.

Tests are relatively trivial to write and probably should be written. The tests could also help serve to demonstrate some of the things that can be done here.

Being able to have a very simple if() is quite valuable, particularly with token parsing.

StatusFileSize
new428 bytes
new24.16 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-1938062-66_1.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Backported the test coverage from drupal 8 to drupal 7: http://drupalcode.org/project/ctools.git/commit/4dc2d93c34ae0a5759dace09...

Just wondering: Does "if(3 > 2, 4, 5)" actually work? The test says no., see math_expression.test at the end.

@todo: Add a test for string support.

The diff looks horrible because ->e( support was dropped.

Hm. ->e() support can be added back in. I just thought it was kind of silly, so removed it. But that's actually a bad idea; clearly people might be using it.

In working with this today I've found quite a few issues with the string support, so I'll have to post a new patch when I get a chance. I also added support for substr() because that's really useful when doing some string manipulation.

StatusFileSize
new10.7 KB
new46.33 KB
FAILED: [[SimpleTest]]: [MySQL] 127 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Here are the files from the right directory.

Status:Needs review» Needs work

The last submitted patch, drupal-1958538-2.patch, failed testing.

tested patch #4, and it can solve the problem.

Tested only the express "MAX" and "MIN".

Component:Code» Documentation

Where should documentation for this function go? In the code, or somewhere here on the Drupal site?

I am currently doing some trial and error and would like to share some of my observations.

Thanks,
r.b.

It seems to be that for ctools advanced help is the place to go?

So... some of this is not ready for the docs page since it is developmental. In answer to @dawehner #2 above, I believe that the answer is "no", the IF code does not yet work. I tested this as follows:

For my test expression I desired a basic switch, keyed on a 0 or a 1 - so if my switch is 1 I return calculation "s" and if it is not (zero is the other setting currently) it returns calculation "c". I have two cases of the IF expression here, the 1st does not work and the 2nd does:

a = if(([field_rate_type] != 1), s, c);
a

This does not work, since the function ctools_math_expr_if is passed a blank character as its first argument a la: ctools_math_expr_if(, 3.97828, 4.3182714126005 ).

However, The IF expression DOES work if it is passed a numeric binary argument, so the following code DOES work:

a = if( [field_rate_type] , s, c);
a

Returning "s" if my field-rate_type = 1 and "c" otherwise. So, what I think is happening is that the TRUE/FALSE result of the comparator "if (($op1 == $op2)) " is being somehow stripped because it is non-numeric?

The following code works in both cases:

         case '==':
            //original code
            //$stack->push($op1 == $op2);
            if (($op1 == $op2)) {
               $stack->push(1);
            } else {
               $stack->push(0);
            }
            break;

I have not formulated this into a patch, as 1) I don't know if returning numerical instead of logical results is the desired result of this IF function, and 2) I am just starting to learn git and don't know how to do a proper patch yet.

Regards,
r.b.

Status:Needs work» Needs review
StatusFileSize
new35.72 KB
FAILED: [[SimpleTest]]: [MySQL] 86 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

The switch from TRUE/FALSE to 0/1 has been applied to the == operator only in this patch. Still dunno if this is correct. Should the module return "pure" logical operators?

Status:Needs review» Needs work

The last submitted patch, drupal-1958538-3.patch, failed testing.

Status:Needs work» Needs review

#10: drupal-1958538-3.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, drupal-1958538-3.patch, failed testing.

StatusFileSize
new35.79 KB
FAILED: [[SimpleTest]]: [MySQL] 120 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Added the e() function as an alias of the evaluate() function so that the tests could execute.

r.b.

Status:Needs work» Needs review
StatusFileSize
new35.79 KB
FAILED: [[SimpleTest]]: [MySQL] 120 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, drupal-1958538-4.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new46.43 KB
FAILED: [[SimpleTest]]: [MySQL] 127 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Repatched from #4...

Status:Needs review» Needs work

The last submitted patch, drupal-1958538-5.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new46.52 KB
FAILED: [[SimpleTest]]: [MySQL] 127 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Added the fix mentioned in #8 to the ">" operator in order to pass the test.

Status:Needs review» Needs work

The last submitted patch, drupal-1958538-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new46.52 KB
PASSED: [[SimpleTest]]: [MySQL] 128 pass(es).
[ View ]

Modified test file, testif in line 141 was doomed to fail I think - since it test if a small # was > a bigger number, but asserted equal to return the larger number instead of the smaller:

    $this->assertEqual($math_expression->evaluate("if($random_number_a > $random_number_b, $random_number_a, $random_number_b)"), $random_number_a);
TO
    $this->assertEqual($math_expression->evaluate("if($random_number_a > $random_number_b, $random_number_a, $random_number_b)"), $random_number_b);

Sorry if this is too much detail, i am monkeying about and being a newby I want to make sure that it is crystal clear what I am goofing up.

r.b.

It would be cool if you could provide an interdiff, as it is way easier to review.

StatusFileSize
new1.55 KB

Thanks for the heads up - interdiff attached.

Thank you! Theoretically we could cast the bool to an int here, but I am fine with that.

We have quite some test coverage to it feels okay to get that one in and maybe even iterate on top of it later?

That's just fine, though it would take me about 15 minutes to duplicate that approach for the remaining comparison operators, making it a more functional patch. I will put that version of the patch up here and then you can decide which you desired to put in.

r.b.

StatusFileSize
new1.75 KB
new46.38 KB
PASSED: [[SimpleTest]]: [MySQL] 128 pass(es).
[ View ]

Attached sets all logical operators to return integer 1/0 using an intval cast.

r.b.