Hi Barney, Jordan, I would like to thank you all again for discussing this RFC in great detail. I believe that the specification for the `BCMath\Number` class is in very good shape due to the detailed discussions and the great amount of feedback. Now that we have been able to brush up the specifications so far, I would be very happy if we could have a good discussion about the concerns that still exist.
> Coming back to this point, I think these are basic features that people would > expect to be there - I think I would find just slightly frustrating to start > learning how to use a class like this and then > find that it doesn't have these functions. Casting and calling `intval` or > `floatval` all feel like slightly awkward workarounds that shouldn't be > needed in a greenfield project. We know that the string > inside the object is always a numeric string, so it should be easier to parse > it as an int than to parse it as a date or a JSON document. Code doing the > latter should stand out as odd looking. > The class cannot guarantee that it can return a value in the type you request > however, so the way that is handled would need to be decided. The value can > easily be outside of the range of an int. Should it return a float silently > in that case for `toInt()`? What if the value is beyond the range of a float? > That would be a very rare situation, as floats can represent extremely large > numbers (with very reduced accuracy), but I would expect it to throw an > exception if that happened. Ideally an exception that I could catch and > ignore, since I can almost surely deal with that error in most situations. > > What about a number that is so small that it can't fit in a float? Similar > situation, though I expect it would occur slightly more often than a number > being too large to fit in a float, even though it would also be rare. > > I think these helper functions belong in the RFC, but they aren't quite > straightforward, which is what I think Saki was alluding to. > Yes I agree there are subtleties to work out for these functions. I don't > think there's such a thing as a number to small to fit in a float. Converting > from decimal to float is always an approximation, sometimes the best > approximation available as a float will be either 0 or -0. If there is a demand for such functionality, I think it should of course be included in the RFC. However, as Jordan mentioned, proper error handling is required. IMHO, I think the following specifications are good: - toInt(): If it has a decimal part, round it to an integer according to the rounding mode. If the result is too large to be represented as an int, it will not be converted to a float and will throw an exception meaning it cannot be converted to an int. - toFloat(): If the result is too large or too small, throws an exception meaning it cannot be converted, just like int. Also, if the value is within the representable range but the precision is too fine, such as `1.0000000000000001`, it will be rounded according to the rounding mode. However, if round to a precision of 17 or 18 digits, the error will end up being very large, so refer to the value defined in `PHP_FLOAT_DIG` and use 15 digits as the maximum number of digits. This constant is 16 for IBM and 15 for others, but for consistency of operation it should be set to 15. >> I don't think it will be possible to make a good Money class as a child of >> this. BcNum is a read-only class, so if the constructor of BcNum is final >> then the child class won't be able to take the currency as a constructor >> param, and won't be able to protect the invariant that currency must be >> initialized. If the constructor of BcNum were made non-final then BcNum >> wouldn't be able to protect the invariant of the numeric value always being >> initialized once the constructor has returned. And it should be >> straightforward to make a good money class as a composition of BcNum and a >> currency enum or string. > > The RFC does not list the constructor as final, and I would not expect it to. >> There's probably not any good use case for a subclass to add properties, >> unless perhaps the subclass developers are willing do do away with some of >> the checks that would normally be done on a value object by the PHP runtime >> to keep it valid (maybe replacing them with static analysis checks). But >> there are lots of reasonable use cases for subclasses to add methods, even >> if they're effectively syntax sugar for static methods with a BcNum typed >> param. > > I literally just provided in the quote you are replying to a good use case > for a subclass. You can do the same thing with composition yeah, and that > might even be better to a lot of people, but I don't think this RFC should > take a position AGAINST subclasses and in favor of composition. >> Having just written that I've now checked the behavior of DateTime - see >> https://3v4l.org/5DQZg. The constructor of that isn't final, so a child >> class can replace it with an empty constructor. But if it does that and >> leaves the value uninitialized then it blows up on a call to `format`. >> That's probably not something we should emulate. DateTimeImmutable behaves >> the same way. > Why not? Writing something like that obviously does not work, and the error > would be immediately apparent. Is it possible to create something that will > error? Of course. That's not an error that needs to be aggressively guarded > against, because the feedback is rapid and obvious. I don't think protecting initialization by the constructor is necessary. https://3v4l.org/YroTN This problem can occur with any class, not just `DateTime`. And I think this is the responsibility of the user's code, and the language just needs to throw an exception appropriately. To clarify: In my opinion, I think that value objects provided by languages are not intended to force immutability or design onto users, to provide functionality to users who expect them to behave as immutable value objects. Therefore, even if a child class that inherits from `Number` behaves in a way that is not immutable, it is the responsibility of the user and there is no need to prohibit it. In other words, the concerns we are currently aware of are issues of responsibility on the user's side, and my opinion is that there is no need to give that much consideration. However, this is my personal opinion, and there may be more reasonable opinions. Regards. Saki
