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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/Rules/ValidEvenNumber.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,20 @@ class ValidEvenNumber implements Rule
*/
public function passes($attribute, $value): bool
{
return preg_match('/^\d*[02468]$/', $value);
$number = strval($value);
$number = explode('.', $number);

if (isset($number[1]) && $number[1] != 0) {
return false;
}

$number = $number[0];

if (extension_loaded('gmp')) {
return gmp_cmp(gmp_mod($number, '2'), '0') === 0;
}

return $number % 2 === 0;
}

/**
Expand Down
32 changes: 32 additions & 0 deletions tests/Rules/ValidEvenNumberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,36 @@ public function test_check_number_is_not_even()

$this->assertFalse($passes);
}

/**
* Test float number is even.
*
* @test
*
* @return void
*/
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

$passes = $this->app['validator']->make($data, $rules)->passes();

$this->assertTrue($passes);
}

/**
* Test float number is not even.
*
* @test
*
* @return void
*/
public function check_float_number_is_not_even()
{
$rules = ['even_number' => [new ValidEvenNumber]];
$data = ['even_number' => '333.13'];
$passes = $this->app['validator']->make($data, $rules)->passes();

$this->assertFalse($passes);
}
}
Loading