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

Check return value for getProduct() in getPrice(). #7794

Merged
merged 3 commits into from
Mar 14, 2017

Conversation

pbaylies
Copy link
Contributor

@pbaylies pbaylies commented Dec 14, 2016

If getProduct() returns a NULL value then this code will generate a PHP warning and potentially crash the page; let's not do that, and check our return values before we try to call methods on them.

If getProduct() returns a NULL value then this code will generate a PHP warning and potentially crash the page; let's not do that, and check our return values before we try to call methods on them.
Copy link
Contributor

@antonkril antonkril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider saving results of getCustomOption() and getProduct() calls. Also please address static test feedback.

Save results of methods called and clean up code in response to feedback and tests.
return $product->getCustomOption('simple_product')->getProduct()->getPrice();
} else {
return 0;
if ( !empty( $product ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is not required

@vrann vrann self-assigned this Mar 6, 2017
@vrann vrann added this to the March 2017 milestone Mar 6, 2017
@vrann
Copy link
Contributor

vrann commented Mar 8, 2017

@pbaylies Thank you for contribution. Can you please fix following code styles issues:

FILE: ...e/Magento/ConfigurableProduct/Model/Product/Type/Configurable/Price.php
--------------------------------------------------------------------------------
FOUND 6 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
 42 | ERROR | Expected 0 spaces after opening bracket; 1 found
 42 | ERROR | Expected 0 spaces before closing bracket; 1 found
 44 | ERROR | Expected 0 spaces after opening bracket; 1 found
 44 | ERROR | Expected 0 spaces before closing bracket; 1 found
 46 | ERROR | Expected 0 spaces after opening bracket; 1 found
 46 | ERROR | Expected 0 spaces before closing bracket; 1 found
--------------------------------------------------------------------------------

@pbaylies
Copy link
Contributor Author

pbaylies commented Mar 8, 2017

@vrann I can't reproduce this; I don't see any extra spaces at the end of those lines on my committed file.

@vrann
Copy link
Contributor

vrann commented Mar 13, 2017

@pbaylies

these lines

if ( !empty( $product ) ) {
if ( !empty( $simpleProductOption ) ) {
if ( !empty( $simpleProduct ) ) {

should look like this:

if (!empty($product)) {
if (!empty($simpleProductOption)) {
if (!empty($simpleProduct)) {

Coding style updates.
@pbaylies
Copy link
Contributor Author

@vrann Done, thanks!

@vrann
Copy link
Contributor

vrann commented Mar 14, 2017

@pbaylies Thank you!

@magento-team magento-team merged commit cc97c1b into magento:develop Mar 14, 2017
@vrann
Copy link
Contributor

vrann commented Mar 14, 2017

@pbaylies merged!

hostep pushed a commit to hostep/magento2 that referenced this pull request Sep 2, 2017
…value for getProduct() in getPrice(). magento#7794

 - Merge Pull Request magento#7794 from pbaylies/magento2:patch-2

(cherry picked from commit 6752968)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants