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

[1.x] Improvment even number rule performance #100

Conversation

amirhossein-fzl
Copy link
Contributor

1- Improved ValidEvenNumber rule performance
2 - Fixed the issue with decimal numbers

public function check_float_number_is_even()
{
$rules = ['even_number' => [new ValidEvenNumber]];
$data = ['even_number' => '754.00'];

Choose a reason for hiding this comment

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

I’ve recently noticed some unfairness toward this Repo, but I sincerely appreciate you submitting this PR and supporting it.
My question is: Does checking for even or odd in decimal numbers have any specific meaning in mathematics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. It doesn't mean anything. But in your code, the same number fails the test... You can try. Even though the number is even....

Choose a reason for hiding this comment

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

But in your code, the same number fails the test

Therefore, the test that has been written does not provide adequate coverage.
First, the test should be refactored to address this issue.
The best approach would be to use data providers in the PHPUnittest to cover a variety of values effectively.

Copy link
Contributor Author

@amirhossein-fzl amirhossein-fzl Feb 12, 2025

Choose a reason for hiding this comment

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

I apologize. Does the current test provide adequate coverage?? If it did, I think the code on decimal numbers should not be in trouble...

What exactly is your idea? Can you provide a reference?

Choose a reason for hiding this comment

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

Does the current test provide adequate coverage??

No, the tests are not sufficient. For example, you are using gmp, but there are no tests for large numbers, or for cases like 2abc and ...

Do we need to write a separate test for each case? The answer is NO.
You can use data providers to cover all these cases with a single method. Please refer to the link below.

https://docs.phpunit.de/en/10.5/writing-tests-for-phpunit.html#data-providers

@milwad-dev milwad-dev changed the title Improvment even number rule performance [1.x] Improvment even number rule performance Feb 13, 2025
Copy link
Owner

@milwad-dev milwad-dev left a comment

Choose a reason for hiding this comment

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

Thanks!

@milwad-dev milwad-dev merged commit 814d78b into milwad-dev:1.x Feb 13, 2025
19 checks passed
public function check_float_number_is_even()
{
$rules = ['even_number' => [new ValidEvenNumber]];
$data = ['even_number' => '754.00'];

Choose a reason for hiding this comment

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

Does the current test provide adequate coverage??

No, the tests are not sufficient. For example, you are using gmp, but there are no tests for large numbers, or for cases like 2abc and ...

Do we need to write a separate test for each case? The answer is NO.
You can use data providers to cover all these cases with a single method. Please refer to the link below.

https://docs.phpunit.de/en/10.5/writing-tests-for-phpunit.html#data-providers

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

Successfully merging this pull request may close these issues.

3 participants