[wp-trac] [WordPress Trac] #30891: Unchecked property overloading is detrimental to OOP.

[WordPress Trac]

#30891: Unchecked property overloading is detrimental to OOP.
--------------------------+------------------
 Reporter:  aercolino     |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  4.2
Component:  General       |     Version:  4.0
 Severity:  normal        |  Resolution:
 Keywords:                |     Focuses:
--------------------------+------------------

Comment (by jdgrimes):

 Replying to [comment:6 wonderboymusic]:
 > In [changeset:"31133"]:
 > {{{
 > #!CommitTicketReference repository="" revision="31133"
 > In `Custom_Background`:
 >
 > * In [28481], `$admin_header_callback` and `$admin_image_div_callback`
 were set to `private` based on their erroneous `@param` value
 > * `$admin_header_callback` and `$admin_image_div_callback` are used as
 hook callbacks - as such, they must be `public`
 > * In [28521] and [28524], magic methods were added for back-compat
 > * Currently, there are 2 properties marked `private`, `$page` and
 `$updated` - `$page` is never used and `$updated` was added by me in
 [30186] during 4.1
 >
 > Set `$admin_header_callback` and `$admin_image_div_callback` to
 `public`.
 > Remove the `$page` property - it duplicated the `$page` local var and is
 referenced/used nowhere.
 > Remove the magic methods - they were beyond overkill and rendered moot
 by the above changes.
 >
 > See #30891.
 > }}}

 I think this should be reverted. Reasoning:

 * The `$admin_header_callback` and `$admin_image_div_callback` don't need
 to be public, because it isn't themselves that is hooked to an action, but
 their values. If this was true we'd have experienced Fatal errors all over
 the place.
 * The `$page` property should be deprecated, but kept for backward
 compatibility.

 Correct me if I'm wrong. :-)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/30891#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform