Skip to content
This repository has been archived by the owner on Feb 3, 2022. It is now read-only.

query regarding rate result method loop #72

Open
joolswills opened this issue Aug 15, 2016 · 4 comments
Open

query regarding rate result method loop #72

joolswills opened this issue Aug 15, 2016 · 4 comments

Comments

@joolswills
Copy link
Contributor

joolswills commented Aug 15, 2016

in app/code/community/Meanbee/Royalmail/Model/Shipping/Carrier/Royalmail.php you have

            foreach ($allowedMethods as $allowedMethod) {
                foreach ($calculatedMethods as $methodItem) {
                    if ($allowedMethod[1] == $methodItem->shippingMethodNameClean) {

is that required ? I was testing with just a single loop

                foreach ($calculatedMethods as $methodItem) {

and that seemed to produce the same results ? (Or are there cases where it wouldn't )

@joolswills
Copy link
Contributor Author

In my fork I have change the loop order, as calculated methods is in order of price, and we wanted to give free postage only on the cheapest option, hence I was wondering about this code.

@joolswills
Copy link
Contributor Author

joolswills commented Aug 15, 2016

it was just a fluke the order seemed right after switching - I see the order looks like that of the CSV so I will do a sort (but still require the loop swapped).

@joolswills
Copy link
Contributor Author

To sort them I am doing

            uasort($calculatedMethods, function($a, $b) {
                return $a->methodPrice > $b->methodPrice;
            });

            foreach ($calculatedMethods as $methodItem) {
                foreach ($allowedMethods as $allowedMethod) 

so it's simple to just offer the first one free in the loop.

@RidRack
Copy link
Contributor

RidRack commented Sep 16, 2016

That extra loop through allowedMethods is to account for the System Configuration values, where methods might be enabled or disabled. Users might have deselected options and this should account for that. Good idea to sort them though, that should speed the loop.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants