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

Simplify Type casting #142

Merged
merged 5 commits into from
Oct 29, 2022
Merged

Simplify Type casting #142

merged 5 commits into from
Oct 29, 2022

Conversation

leemyongpakvn
Copy link
Contributor

@leemyongpakvn leemyongpakvn commented Jun 19, 2022

Questions Answers
Description? 1. Assign $id_xyz = (int) $id_xyz once at top, instead of using (int) $id_xyz throughout the function
2. if Validate::isUnsignedId($id_xyz ) then there is no need to type cast (int) ($id_xyz)
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#28879.
How to test? Check BO Configuration form and FO Product page and Category page where ProductComments is used.

1. Assign $id_xyz = (int) $id_xyz once at top, instead of using (int) $id_xyz throughout the function
2. if Validate::isUnsignedId($id_xyz ) then there is no need to type cast (int) ($id_xyz)
if Validate::isUnsignedId($id_product) then there is no need to type cast (int) $id_product
leemyongpakvn and others added 2 commits June 20, 2022 17:33
Co-authored-by: Matthieu Rolland <matthieu.rolland@prestashop.com>
@@ -95,27 +95,28 @@ public static function getByProduct($id_product, $p = 1, $n = null, $id_customer
$validate = (bool) Configuration::get('PRODUCT_COMMENTS_MODERATE');
$p = (int) $p;
$n = (int) $n;
$id_customer = (int) $id_customer;

Choose a reason for hiding this comment

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

Why not using Validate::isUnsignedId as few lines above for $id_product ?

Copy link
Contributor Author

@leemyongpakvn leemyongpakvn Jul 4, 2022

Choose a reason for hiding this comment

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

At line 90, we have $id_customer is an optional parameter and is assigned to null if omitted.
I'm not sure about the logic of that case so I just let it there.

@kpodemski kpodemski merged commit 0718b6a into PrestaShop:dev Oct 29, 2022
@kpodemski
Copy link
Contributor

Thanks @leemyongpakvn

I didn't notice any regressions during the tests.

@leemyongpakvn leemyongpakvn deleted the patch-1 branch November 2, 2022 13:16
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.

Duplicate type casting in productcomments module
6 participants