Hi all,
I would like to start additional discussion about BCMath\Number in this thread. If my summary is incorrect, please let me know.
First, make a list of points to note.
- Suggestions for using 64-bit integer types internally for performance
If we are not going to use the newfound opaque type to introduce performance optimization for the common cases of small numbers that fit into 64-bit integers, then the value of adding a dedicated object is vastly diminished.
Python is literally doing this sort of optimizations to improve the speed of their arbitrary precision integer type for the common use case of "small integers".In general, if it cannot fit in a native type then it should continue to use a string, but I would expect the vast majority of cases to fit in a 64-bit integer.
IMHO: I agree that it can be computed fairly quickly if the conditions are right. However, I believe that this itself does not require an RFC and can be addressed as part of a performance improvement.
- Points out regarding naming, such as changing "powmod" to “powMod”
IMHO: I simply copied the existing naming, so I have no objection to using the correct naming conventions.
- Points out regarding comparison methods
In general, I am not sure why there is a need in the first place to expose any of the overloaded operators as methods.
This includes the add/sub/div/mult/mod methods, but I can be convinced that this makes some sense.I was planning on tackling our rather broken comparison semantics [1] this year, but got distracted by the broken container/offset semantics instead. [2]
As, IMHO, we need to fix the internal object handlers and semantics first before giving this power to userland.
And I feel strongly that using instance methods for overload is the "wrong" way of doing it.
Moreover, if we do provide the option for userland to overload they should, again IMHO, just be able to overload == and <=>, not all the individual comparison operators.
Thus, providing all those methods makes no sense to me.Moreover, taking inspiration from userland for naming methods because they cannot overload operators seems backwards.
Userland classes, if they want to provide such functionality, must implement a wide range of methods, however an internal class does is not required to do this.
Therefore, I am not really convinced by following suit on userland nomenclature.
IMHO: Indeed, as long as have compare, all the functions of the comparison method can be fulfilled. This is related to "Methods to be published" described later. These are exactly the I don't know if I'll use it, but I added it because it seemed useful'' method. Certainly, there are no things we can't do without these methods.
However, I see no reason for exposing lt/gt/gte/lte.
I am also not really a fan of the compare and equal methods to be instance methods compared to static methods.
Adding those methods might interfere with any potential RFC that adds an Equatable/Ordable interface to allow userland classes to overload the comparison operators.
I think this is a type of functionality called “procedural".
- Whether existing BCMath functions should accept BCMath\Number
IMHO: As Derick also mentioned, scale is handled differently, which can lead to confusion.
- About the behavior when casting BCMath\Number to bool
IMHO: I like it like int where 0 is false and everything else is true.
- Regarding whether the method you plan to publish is really necessary
Note: For example, the format() method only supports the notation methods of a limited number of countries, so its functionality is incomplete. In addition, it has been pointed out that it would be better not to expose many functions from the beginning, but to keep them to the minimum necessary first, and add some methods later.
IMHO: Admittedly, I may have crammed in too many features to make it useful. Which method do you think is the minimum required?
Regard,
Saki
Hi!
This is a reminder as the feature freeze is approaching. I would like to restate my current opinion.
- Suggestions for using 64-bit integer types internally for performance
This is probably unnecessary. It's already fast enough, so it might be a good idea to consider it when we need even more speed. Also, if we want to speed things up even more, it would probably be better to change the design of the bc_num structure.
- Points out regarding naming, such as changing "powmod" to “powMod”
A function called "divmod" is also commonly used in other languages. If follow that naming, "powmod" is not unnatural. Since "bcpowmod" already exists, I feel that it is okay to leave it as "powmod”.
- Points out regarding comparison methods
For now, all comparisons are possible with just "compare", so I'll use only one comparison method. This requires an override RFC.
- Whether existing BCMath functions should accept BCMath\Number
At least not in 8.4. I don't think any ideas for how to use scale properly will be finalized before the feature freezes.
- About the behavior when casting BCMath\Number to bool
If the value is 0, it is treated as false, otherwise it is treated as true. This alone may not require an RFC, but I'm planning to create an RFC for the comparison method, so I'll include it there.
- Regarding whether the method you plan to publish is really necessary
I decide to delete the format method. We can cast or get the value from the property. In some cases, we may consider adding it again, but at least not in 8.4. This also requires an RFC.
Regards,
Saki
Hi!
This is a reminder as the feature freeze is approaching. I would like to restate my current opinion.
Apologies for the delay, to much traffic on the list so I forgot to address this.
- Suggestions for using 64-bit integer types internally for performance
This is probably unnecessary. It's already fast enough, so it might be a good idea to consider it when we need even more speed. Also, if we want to speed things up even more, it would probably be better to change the design of the bc_num structure.
ACK, this seems to be fine.
- Points out regarding naming, such as changing "powmod" to “powMod”
A function called "divmod" is also commonly used in other languages. If follow that naming, "powmod" is not unnatural. Since "bcpowmod" already exists, I feel that it is okay to leave it as "powmod”.
- Points out regarding comparison methods
For now, all comparisons are possible with just "compare", so I'll use only one comparison method. This requires an override RFC.
I'm grouping those points together.
My main concern with adding those methods to the class is that this will potentially conflict with any implementation of operator and/or comparison overloading.
This is why I would rather not have the methods at all, if the idea is to have a "procedural" API I feel revisiting Andrea's old "Operator functions" RFC would be better.
https://wiki.php.net/rfc/operator_functions
- Whether existing BCMath functions should accept BCMath\Number
At least not in 8.4. I don't think any ideas for how to use scale properly will be finalized before the feature freezes.
ACK, the context is also different than for ext/gmp which I misremembered.
- About the behavior when casting BCMath\Number to bool
If the value is 0, it is treated as false, otherwise it is treated as true. This alone may not require an RFC, but I'm planning to create an RFC for the comparison method, so I'll include it there.
I think this is the most sensible, in which case I think GMP should also follow suit and have the same semantics instead of currently being broken when you try to cast to bool.
- Regarding whether the method you plan to publish is really necessary
I decide to delete the format method. We can cast or get the value from the property. In some cases, we may consider adding it again, but at least not in 8.4. This also requires an RFC.
Seems sensible to me.
Best regards,
Gina P. Banyard
Hi Gina,
Apologies for the delay, to much traffic on the list so I forgot to address this.
Don’t worry!
- Suggestions for using 64-bit integer types internally for performance
This is probably unnecessary. It's already fast enough, so it might be a good idea to consider it when we need even more speed. Also, if we want to speed things up even more, it would probably be better to change the design of the bc_num structure.
ACK, this seems to be fine.
- Suggestions for using 64-bit integer types internally for performance
This is probably unnecessary. It's already fast enough, so it might be a good idea to consider it when we need even more speed. Also, if we want to speed things up even more, it would probably be better to change the design of the bc_num structure.
ACK, this seems to be fine.
- Regarding whether the method you plan to publish is really necessary
I decide to delete the format method. We can cast or get the value from the property. In some cases, we may consider adding it again, but at least not in 8.4. This also requires an RFC.
Seems sensible to me.
OK, thanks!
- Points out regarding naming, such as changing "powmod" to “powMod”
A function called "divmod" is also commonly used in other languages. If follow that naming, "powmod" is not unnatural. Since "bcpowmod" already exists, I feel that it is okay to leave it as "powmod”.
- Points out regarding comparison methods
For now, all comparisons are possible with just "compare", so I'll use only one comparison method. This requires an override RFC.
I'm grouping those points together.
My main concern with adding those methods to the class is that this will potentially conflict with any implementation of operator and/or comparison overloading.
This is why I would rather not have the methods at all, if the idea is to have a "procedural" API I feel revisiting Andrea's old "Operator functions" RFC would be better.
https://wiki.php.net/rfc/operator_functions
First of all, am I correct in understanding that there is no problem with the naming of "powmod”?
Also, given that I have implemented only one "compare()" method, how could this conflict with them? Number is a final class, so there is no room for the user to change its behavior.
- About the behavior when casting BCMath\Number to bool
If the value is 0, it is treated as false, otherwise it is treated as true. This alone may not require an RFC, but I'm planning to create an RFC for the comparison method, so I'll include it there.
I think this is the most sensible, in which case I think GMP should also follow suit and have the same semantics instead of currently being broken when you try to cast to bool.
That makes sense. However, I would like to have a separate vote on GMP changes. For procedural efficiency, we will combine them into one RFC, but each will be voted on separately, and each requires a 2/3 or more vote in favor. In other words, two primary votes will be held at the same time.
Regards,
Saki