Hi Gina, > I voted in favour, as Number being its own type should improve DX and > performance for small numbers as we can just store two 64bit integer, one for > the integral and another for the fractional part. > However, I have some remarks and questions. > Apologies if these were already brought up, but I haven't gone through the > whole 100 long email thread.
First of all, I have to clear up your misconception. Number is an object that uses a structure held internally by BCMath. Therefore, the data structure will be the same as the existing BCMath. In other words, supporting object types does not dramatically improve performance or introduce new computational logic. Even if the data structure is different from the existing one, just two 64-bit values are probably not enough for BCMath. This may be off-topic, as you may know, BCMath is, in my opinion, a type of what is called BCD (Binary-coded decimal). If we expect to improve performance while maintaining these characteristics, we may want to consider a data type called DPD (densely packed decimal). (I'm doing some refactoring right now, but the changes to the data structure will be quite extensive, so I haven't thought much about it yet.) > - The comparison method should either be called "cmp", or "compare" to align > with other extensions, the BC Math function being called bc_comp is weird. > - The "powmod" method should probably be called "powMod" I named to match existing function names [1]. Admittedly, these names may seem a little strange. However, I didn't change this because it would create a new debate about consistency with existing BC function naming and complicate the RFC. > - I don't understand why we are exposing lt, lte, gt, and gte what is the > rationale here? Also, the naming is kinda terrible IMHO. Does that mean it's unnecessary because it can be compared by operator overload? Or it's unnecessary because `comp()` exists? Regarding naming: These names are also used in Carbon, a php date and time library (these are shorthand name methods) [2]. It is possible to name something like `greaterThanOrEqualTo`, but I personally felt it was a little too long, so I shortened it. Names like these are also used in Laravel's validation rules. Therefore, for PHP users, at least I don't think it's something that makes no sense when see it for the first time. > - Are the existing BCMath function going to be adapted to handle the new > Number object or not? No, that is not considered at this time. > - What is the behaviour when casting a Number object to bool? Does it always > cast to true like a "standard" object, or does a Number equal to 0 cast to > false? That's a good topic. I didn't include any mention of this in the RFC because there was no discussion about it. Personally, I like to return a bool value that reflects the internal numerical state, like an int. However, since I would like to avoid adding such specifications later when voting has already begun, I will use the same as a "standard object" for now. If this RFC is passed, I will additionally propose it in another RFC. > - Considering the property hook RFC has been accepted, should the $value > property be a "virtual" property or not? I think that's probably so. But my understanding is that in this case I don't think it makes any difference to the user. [1] https://www.php.net/manual/ja/function.bccomp.php [2] https://carbon.nesbot.com/docs/#api-comparison Regards, Saki
