News:

Looking for documentation? Take a look on our wiki

Main Menu

Code Enhancement Request - VmCalculation

Started by virtuemartaustralia, January 10, 2018, 04:47:08 AM

Previous topic - Next topic

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 .

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
www.virtuemartoz.com
Shipping: AusPost NZPost
Payment: eWay, SecurePay, merchantwarrior, ematters, paymentexpress
Infinite Scroll,Category Discount,wholesale Discount
Westfield Integration, Make An Offer/Best Offer
We do things others can't do, call Us For A hand of help.

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.
Should I fix your bug, please support the VirtueMart project and become a member
______________________________________
Extensions approved by the core team: http://extensions.virtuemart.net/