Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimized Cart class #5579

Merged
merged 1 commit into from May 23, 2016

Conversation

@mickaelandrieu
Copy link
Member

commented May 17, 2016

Questions Answers
Branch? develop
Description? Put a cache on total shipping cost as they cant change during a request
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
How to test? https://blackfire.io/profiles/compare/d97f00b6-8d8e-4c30-b55c-bd5d36dc4d02/graph
@Shudrum

View changes

classes/Cart.php Outdated
@@ -92,6 +92,7 @@ class CartCore extends ObjectModel
protected static $_carriers = null;
protected static $_taxes_rate = null;
protected static $_attributesLists = array();
protected static $_total_shipping = array();

This comment has been minimized.

Copy link
@Shudrum

Shudrum May 19, 2016

Contributor

I think that for memoized vars, it is a way better pratice to just declare static $totalShipping; at the beginning of the public method.

It is better for readability, but way more important, it avoid an external modification of the value.

@Shudrum

This comment has been minimized.

Copy link
Contributor

commented May 19, 2016

👍 Yeah!

@mickaelandrieu mickaelandrieu force-pushed the mickaelandrieu:optimized-cart branch to d4f536d May 23, 2016

@Shudrum Shudrum merged commit c2fb7c1 into PrestaShop:develop May 23, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@mickaelandrieu mickaelandrieu deleted the mickaelandrieu:optimized-cart branch Sep 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.