Code Enhancement Request - VmCalculation

[virtuemartaustralia]

Hello ,
Recently i've played around with vmcalculation again, and see there is potential improvement on the code,
Here is a scenario,
I have two vmcalculation plugins, pluginA, pluginB , price modifier is 'Discount before tax per product', in their effective function interpretMathOp, they all have conditions to check if it's their duty to return a discounted price, if no then return the original $price which passed in by value of interpretMathOp, when the rule of cart's content matches pluginB, but does not match pluginA, the final price returned should be discountB, but in the current VM's code, it returned an array ($price, $discountB), and it returned the first element $price as the final result.
$price is from pluginA, $discountB if from pluginB, but $price is useless in such a situation.
It will work when there's only ONE plugin in the SAME price modifier GROUP.
Only return NULL in pluginA and return $discountB in above scenario will solve the problem, but it creates another problem, when cart contents neither match pluginA or pluginB,  it yields a vm error message because the caller is expecting a positive $price value.
My advice is to use the passed in parameter $price in return result, and WRITE in documentation that the $price parameter should be as &$price
So multiple vmcalculation plugins in the same price modifier group will work .

Code Select Expand


JPluginHelper::importPlugin('vmcalculation');
$dispatcher = JDispatcher::getInstance();

$calculated = $dispatcher->trigger('plgVmInterpreteMathOp', array($this, $rule, $price,$this->_revert));
if($calculated){
foreach($calculated as $calc){
if($calc) return $calc;
}
} else {
VmError('Unrecognised mathop '.$mathop.' in calculation rule found, seems you created this rule with plugin not longer accesible (deactivated, uninstalled?)');
return $price;
}



#2 Another code feature request is add an optional class member in CalculationHelper, $currentInterpretingProduct. currently the func interpretMathOp has no idea of which product it's processing, it could be very useful when store owners want to give specific discount per product based on customer's activities.  I wanted to suggest add an optional parameter in interPreteMathOp but in the afterthought that way is better and cleaner.  Just add two not harmful compatible lines in the beginning of your foreach loop in calculationh.php

Virtuemart 3.2.12 Joomla 3.8.2

[Milbo]

I think a better idea is to give a linked $price with &$price.

The current product is already there and just called "_product", yeh it is not declared in the class. I should do that.