Skip to content

ext/bcmath: Added scale to bc_compare argument #14802

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

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

SakiTakamachi
Copy link
Member

In the original specification, the scale of bc_num was directly changed and compared.
This becomes a problem when objects are supported, so we will modify it to compare without changing bc_num.

In the original specification, the scale of bc_num was directly changed
and compared.

This becomes a problem when objects are supported, so we will modify it
to compare without changing bc_num.
@SakiTakamachi SakiTakamachi marked this pull request as ready for review July 3, 2024 23:47
@SakiTakamachi
Copy link
Member Author

@Girgias @nielsdos

This may have been buried🙏

@@ -39,7 +39,7 @@
than N2 and +1 if N1 is greater than N2. If USE_SIGN is false, just
compare the magnitudes. */

bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, bool use_sign)
bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, size_t scale, bool use_sign)
Copy link
Member

Choose a reason for hiding this comment

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

The new scale parameter is unused?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it used on L76 and 77.

@nielsdos
Copy link
Member

I'm not sure I understand (yet) why this is needed.

@SakiTakamachi
Copy link
Member Author

@nielsdos

If specify a scale to compare values ​​on a smaller scale than they should be, must change bc_num->n_scale.

I don't want to do this with BcMath\Number, which is an immutable object, so I want to be able to specify the scale without changing n_scale.

@SakiTakamachi SakiTakamachi merged commit be4b10e into php:master Jul 11, 2024
11 checks passed
@SakiTakamachi SakiTakamachi deleted the refactor_bccomp branch July 11, 2024 15:08
@SakiTakamachi
Copy link
Member Author

thx!

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.

3 participants