Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:122955 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id F2D9A1A009C for ; Fri, 5 Apr 2024 06:41:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1712299303; bh=4wqwAJiKAJCrTYys/OqWpZi6rz0WReoQLctTXIsWnlI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Nwsh0vvV8pyEOhCf6ARuAz9yAteP3lpp8MVDWIr6Z1e2KRp7oiu38+ffwSxADiCSR FsL1rksPerDqcN7lzAHXddVZvj4lWP5iELcdMwJhv2pEnfk2xFdmFLHrAlK0vqJtf8 0TpB7qfTthrInM1YrdM4y3PaveJIPwwz2E4zlE+0CmTfPRD8AHl30Ka+oPfdH1aWs0 SC16ZpD5YOD3u12jEmSVm8j22ixJVzteOZEc4umwJjDC31XOPv9rPeL72KIywAUPvO s0G9BQA4R7awteHvASdg81XrOkPr2liXfRZ4DO7UtulkUOJPlBl9ISzsAmFN5LIrAr Q3PbA7/mQQxIg== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id C13E7180069 for ; Fri, 5 Apr 2024 06:41:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: * X-Spam-Status: No, score=1.0 required=5.0 tests=BAYES_50,DMARC_MISSING, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,SPF_HELO_PASS,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from gliadin.co.uk (gliadin.co.uk [80.82.115.221]) by php-smtp4.php.net (Postfix) with ESMTP for ; Fri, 5 Apr 2024 06:41:41 +0000 (UTC) Received: from [192.168.0.17] (hari-18-b2-v4wan-169870-cust740.vm1.cable.virginm.net [92.239.242.229]) by gliadin.co.uk (Postfix) with ESMTPSA id EAE84FA2A06; Fri, 5 Apr 2024 09:41:09 +0300 (MSK) Content-Type: multipart/alternative; boundary="------------7011D0tw27v0PaKFcVC9GiMK" Message-ID: <5876c04a-bfed-4a97-be65-f8b64ddef31a@redmagic.org.uk> Date: Fri, 5 Apr 2024 07:41:08 +0100 Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath To: Saki Takamachi , Jordan LeDoux Cc: internals@lists.php.net References: <4F094EDA-5058-407D-AF39-06FD934FDE1F@sakiot.com> <904197f4-afb5-401e-9e17-7a655c5449d0@alec.pl> <655FEA80-9AB4-4AAD-A310-70ED968C97A2@sakiot.com> <8FB87901-02D7-4934-9119-55B21CEDDA9D@sakiot.com> <65e0fc8b-ed4e-4bf8-af2d-77dc7b4bb934@redmagic.org.uk> <92F7B1F2-67EE-4276-ADE0-20181B57DF5E@sakiot.com> Content-Language: en-US In-Reply-To: <92F7B1F2-67EE-4276-ADE0-20181B57DF5E@sakiot.com> From: barney@redmagic.org.uk (Barney Laurance) This is a multi-part message in MIME format. --------------7011D0tw27v0PaKFcVC9GiMK Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 05/04/2024 01:20, Saki Takamachi wrote: >>> 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 - seehttps://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. Ah ok. Maybe that's the standard way internal classes are written, and we can consider it generally an error for a child class to override the constructor without calling `parent::__construct()`. I know tools warn users not to forget the parent call. And I had been thinking of BcNum as a readonly class, but I see it's not an readonly class, it's a mutable class with its one and only property being readonly.  I agree that it seems unnecessary to force child classes to be immutable - I wasn't concerned about them adding additional, mutable, properties, only about leaving the value property uninitialized. (Btw I would have supported the 1st part of the 2022 Readonly amendments RFC, which I think would have made a readonly class behave the same as a class where every property is readonly, particularly since would have allowed implementing the example given on the Wikipedia page for LSP: a circle with fixed centre and mutable radius may inherit from an immutable point) --------------7011D0tw27v0PaKFcVC9GiMK Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit


On 05/04/2024 01:20, Saki Takamachi wrote:

      

        

          
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.


    

    


    

    
Ah ok. Maybe that's the standard way internal classes are
      written, and we can consider it generally an error for a child
      class to override the constructor without calling
      `parent::__construct()`. I know tools warn users not to forget the
      parent call.

      

      And I had been thinking of BcNum as a readonly class, but I see
      it's not an readonly class, it's a mutable class with its one and
      only property being readonly.  I agree that it seems unnecessary
      to force child classes to be immutable - I wasn't concerned about
      them adding additional, mutable, properties, only about leaving
      the value property uninitialized. (Btw I would have supported the
      1st part of the 2022 Readonly amendments RFC, which I think would
      have made a readonly class behave the same as a class where every
      property is readonly, particularly since would have allowed
      implementing the example given on the Wikipedia page for LSP: a
      circle with fixed centre and mutable radius may inherit from an
      immutable point)

    

  


--------------7011D0tw27v0PaKFcVC9GiMK--