(*PATCH*) getters/setters Implementation

13 years ago by Clint M Priest — view source — reply

unread

Per RFC:
https://wiki.php.net/rfc/propertygetsetsyntax

Detailed Implementation Notes Document:
https://wiki.php.net/rfc/propertygetsetsyntax-as-implemented

Getters/Setters has been built. This is my first patch to the php core. Here is what has been implemented:

Static getters/setters syntax
static public $Hours {
get { return self::$_Hours; }
set { self::$_Hours = $value; }
}

Asymmetrical getters/setters syntax
public $Hours {
get { return $this->_Hours; }
protected set { $this->_Hours = $value; }
}

Default getter/setter implementations (auto-backing)
public $Hours { /* Implements protected $__Hours; /
get; / { return $this->__Hours; }
set; /* { $this->__Hours = $value; }
}

Interfaces (Requires implementation by implementing classes)
private $Hours {
get;
set;
}

Overloading Properties - Properties are inherited and can be over-ridden in child classes provided the parent has not specified final on an accessor.

Traits
No changes necessary, implementation worked without further changes necessary

Errors
Updated errors to refer to getter/setter implementation when related

Note

    This does not interfere with typical __get()/__set() functionality which will take effect whenever there is not a specific accessor available.

    There are also 25 unit tests, testing all of this functionality

    There is also a patch to change the reflection extension to expose new functions for accessors.

What are the next steps to get this added to some future release? Attached is a patch against ~/trunk

13 years ago by Christopher Jones — view source — reply

unread

What are the next steps to get this added to some future release? Attached is a patch against ~/trunk

A couple of brief comments from the sidelines without having followed previous
discussion in detail:

The RFC appears to have open questions e.g about the need for readonly etc keywords
The tests and RFC are out of sync regarding naming, e.g. readonly vs read-only
The RFC makes no mention of backward compatibility issues
Did I miss seeing tests that pass in unexpected values to the methods?
I would expect a larger number of tests overall when the feature is merged/completed.
If these are indeed magic methods they need "__" prefixes, so consider the names
__getter and __setter
I'd suggest biting the github bullet and creating your own PHP fork with your
patches. People will be able to test and you might get more feedback.

--
Email: christopher.jones@oracle.com
Tel: +1 650 506 8630
Blog: http://blogs.oracle.com/opal/

13 years ago by Clint M Priest — view source — reply

unread

Thanks for the feedback, I'll take care of some of these.

What did you mean about the out of sync regarding naming?

With the unexpected values to the methods I'm not sure what you mean, there are no 'expected values' to be passed.

For the auto-backed properties it would be assigned to whatever value was being passed, null or whatever. For the non auto-backed properties it would depend on the user-supplied getter/setter implementation. Am I missing something here?

Regarding the open questions on read-only/write-only I don't think they are strictly necessary any longer. The original RFC had them for enforcing a value to be read only but it would be equivalent of setting an accessor with just a getter and final although it would allow for it to be over-ridden. Are the read-only/write-only tags desired?

I think the test cases that are present are complete, I could not think of any further tests to write or I would have written them, any suggestions?

I'll update the RFC with backward compatibility comments though I believe there are none, anyone else see any backward compatibility issues?

-----Original Message-----
From: Christopher Jones [mailto:christopher.jones@oracle.com]
Sent: Thursday, March 29, 2012 1:14 PM
To: internals@lists.php.net
Subject: Re: [PHP-DEV] (PATCH) getters/setters Implementation

What are the next steps to get this added to some future release?
Attached is a patch against ~/trunk

A couple of brief comments from the sidelines without having followed previous discussion in detail:

The RFC appears to have open questions e.g about the need for readonly etc keywords
The tests and RFC are out of sync regarding naming, e.g. readonly vs read-only
The RFC makes no mention of backward compatibility issues
Did I miss seeing tests that pass in unexpected values to the methods?
I would expect a larger number of tests overall when the feature is merged/completed.
If these are indeed magic methods they need "__" prefixes, so consider the names
__getter and __setter
I'd suggest biting the github bullet and creating your own PHP fork with your
patches. People will be able to test and you might get more feedback.

--
Email: christopher.jones@oracle.com
Tel: +1 650 506 8630
Blog: http://blogs.oracle.com/opal/

13 years ago by Clint M Priest — view source — reply

unread

The patches are applied to this fork if anyone wants to check it out:

https://github.com/cpriest/php-src

-----Original Message-----
From: Clint M Priest [mailto:cpriest@zerocue.com]
Sent: Thursday, March 29, 2012 8:14 PM
To: internals@lists.php.net
Subject: RE: [PHP-DEV] (PATCH) getters/setters Implementation

Thanks for the feedback, I'll take care of some of these.

What did you mean about the out of sync regarding naming?

With the unexpected values to the methods I'm not sure what you mean, there are no 'expected values' to be passed.

I think the test cases that are present are complete, I could not think of any further tests to write or I would have written them, any suggestions?

I'll update the RFC with backward compatibility comments though I believe there are none, anyone else see any backward compatibility issues?

What are the next steps to get this added to some future release?
Attached is a patch against ~/trunk

A couple of brief comments from the sidelines without having followed previous discussion in detail:

The RFC appears to have open questions e.g about the need for readonly etc keywords
The tests and RFC are out of sync regarding naming, e.g. readonly vs read-only
The RFC makes no mention of backward compatibility issues
Did I miss seeing tests that pass in unexpected values to the methods?
I would expect a larger number of tests overall when the feature is merged/completed.
If these are indeed magic methods they need "__" prefixes, so consider the names
__getter and __setter
I'd suggest biting the github bullet and creating your own PHP fork with your
patches. People will be able to test and you might get more feedback.

--
Email: christopher.jones@oracle.com
Tel: +1 650 506 8630
Blog: http://blogs.oracle.com/opal/

13 years ago by Alexey Shein — view source — reply

unread

31 марта 2012 г. 18:19 пользователь Clint M Priest
cpriest@zerocue.com написал:

The patches are applied to this fork if anyone wants to check it out:

https://github.com/cpriest/php-src

It would be easier to discuss/review your patch if you'd make pull
request: https://wiki.php.net/vcs/gitworkflow#workflow_for_external_contributors
Thank you.

-----Original Message-----
From: Clint M Priest [mailto:cpriest@zerocue.com]
Sent: Thursday, March 29, 2012 8:14 PM
To: internals@lists.php.net
Subject: RE: [PHP-DEV] (PATCH) getters/setters Implementation

Thanks for the feedback, I'll take care of some of these.

What did you mean about the out of sync regarding naming?

With the unexpected values to the methods I'm not sure what you mean, there are no 'expected values' to be passed.

For the auto-backed properties it would be assigned to whatever value was being passed, null or whatever. For the non auto-backed properties it would depend on the user-supplied getter/setter implementation. Am I missing something here?

Regarding the open questions on read-only/write-only I don't think they are strictly necessary any longer. The original RFC had them for enforcing a value to be read only but it would be equivalent of setting an accessor with just a getter and final although it would allow for it to be over-ridden. Are the read-only/write-only tags desired?

I think the test cases that are present are complete, I could not think of any further tests to write or I would have written them, any suggestions?

I'll update the RFC with backward compatibility comments though I believe there are none, anyone else see any backward compatibility issues?

-----Original Message-----
From: Christopher Jones [mailto:christopher.jones@oracle.com]
Sent: Thursday, March 29, 2012 1:14 PM
To: internals@lists.php.net
Subject: Re: [PHP-DEV] (PATCH) getters/setters Implementation

What are the next steps to get this added to some future release?
Attached is a patch against ~/trunk

A couple of brief comments from the sidelines without having followed previous discussion in detail:

The RFC appears to have open questions e.g about the need for readonly etc keywords

The tests and RFC are out of sync regarding naming, e.g. readonly vs read-only

The RFC makes no mention of backward compatibility issues

Did I miss seeing tests that pass in unexpected values to the methods?

I would expect a larger number of tests overall when the feature is merged/completed.

If these are indeed magic methods they need "__" prefixes, so consider the names
__getter and __setter

I'd suggest biting the github bullet and creating your own PHP fork with your
patches. People will be able to test and you might get more feedback.

--
Email: christopher.jones@oracle.com
Tel: +1 650 506 8630
Blog: http://blogs.oracle.com/opal/

--

--

--

--
Regards,
Shein Alexey