Drupal\Core\Datetime\Entity\DateFormat class variables should not be accessed directly. Functions should be use to access the variable. For instance use getDescription() and setDescription($description) for the protected class variable description. For a boolean variable the getter function becomes isVariableName(). In object-oriented programming this is called encapsulation.

Remaining tasks

  • Update the class variables and make them protected.
  • Create getters and setters for frequently used get and set functionality.
  • Update drupal to use the getters and setters instead of accessing variables directly.
  • There are no tests required because the added functions are only getters and setters.

For more info over what should be done see the issue summary of #2016679: Expand Entity Type interfaces to provide methods, protect the properties.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because properties should not be public, API methods should not be allowed to be sidestepped.
Prioritized changes Prioritized since it is a bug and it reduces fragility.
Disruption Somewhat disruptive for core as well as contributed and custom modules:
  • BC break for anything using the public properties: code will need to convert to the methods
  • BC break for anything (mis)using properties that should not really be public: will require minor refactoring
  • BC break for alternate implementations of a given entity interface (rare/probably nonexistent): they will need to implement the new methods

But impact will be greater than the disruption, so it is allowed in the beta.

CommentFileSizeAuthor
#1 2527076-1.patch598 bytesdaffie

Comments

daffie’s picture

daffie commented
Status: Active » Needs review
StatusFileSize
new 2527076-1.patch598 bytes

Setting the two class variables that are not protected to protected. The variable are $id and $label.

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs review » Reviewed & tested by the community

Looks good. As the id() and label() are already defined in parent class, we can still get the value of these properties.

daffie’s picture

daffie commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Talked to catch on IRC and this issue can go in before 8.0. Only there is a change record necessary.

daffie’s picture

daffie commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs change record

Change record added. Back to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Given that this is the last set of public properties on a config entity that really shouldn't be I think that this is okay to proceed with at this point in the beta.

Committed dfa5621 and pushed to 8.0.x. Thanks!

  • alexpott committed dfa5621 on 8.0.x 
    Issue #2527076 by daffie: Make the class variables protected for Drupal\...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.