Skip to content

feat: add new setter/getter for Entity - #7230

Merged
kenjis merged 2 commits into
codeigniter4:4.4from
kenjis:feat-entity-new-setter
Feb 27, 2023
Merged

kenjis merged 2 commits into
codeigniter4:4.4from
kenjis:feat-entity-new-setter

Conversation

@kenjis

@kenjis kenjis commented Feb 7, 2023

Copy link
Copy Markdown
Member

Needs #7208

Description
Add special getter/setter to Entity to avoid method name conflicts.

  • add new special setter method "_set + key" for a attribute
  • add new special getter method "_get + key" for a attribute

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added enhancement PRs that improve existing functionalities 4.4 labels Feb 7, 2023
@kenjis
kenjis force-pushed the feat-entity-new-setter branch from 4ca42db to e977ae9 Compare February 13, 2023 23:51
@kenjis kenjis added the docs needed Pull requests needing documentation write-ups and/or revisions. label Feb 13, 2023
@MGatner

MGatner commented Feb 15, 2023

Copy link
Copy Markdown
Member

I'm not sure what this is needed for? bar:foo

@kenjis

kenjis commented Feb 15, 2023

Copy link
Copy Markdown
Member Author

For method name collision. #5762 (comment)

@MGatner

MGatner commented Feb 16, 2023

Copy link
Copy Markdown
Member

I meant the colons in the value, but now I see those are an artifact of the test fixture, not part of the code! My mistake.

@MGatner MGatner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, pending docs.

@kenjis
kenjis force-pushed the feat-entity-new-setter branch from e977ae9 to 8006f5f Compare February 20, 2023 01:50
@kenjis kenjis removed the docs needed Pull requests needing documentation write-ups and/or revisions. label Feb 20, 2023
@kenjis

kenjis commented Feb 20, 2023

Copy link
Copy Markdown
Member Author

Added docs.

@kenjis
kenjis force-pushed the feat-entity-new-setter branch from 8006f5f to 6f52f1e Compare February 20, 2023 01:54
@kenjis

kenjis commented Feb 20, 2023

Copy link
Copy Markdown
Member Author

@najdanovicivan

Copy link
Copy Markdown
Contributor

I think this adds a lot of unnecessary complexity to have multiple ways of defining setters. So far the only problem we have is when we need to override setAttributes method. Since now we have setAttributes as deprecated and we're going forward with injectRawData() method. I'd rather use https://www.php.net/manual/en/reflectionmethod.getdeclaringclass.php to check if setAttributes is not from CodeIgniter\Entity or CodeIgniter\Entity\Entity class. So if method is not from base CI4 classes it can be handled as regular setter for $this->attributes['attributes']. The only problem with this one is BC in Entity classes which override the setAttributes method.

@kenjis

kenjis commented Feb 21, 2023

Copy link
Copy Markdown
Member Author

@najdanovicivan Thank you for your opinion.

The main reason of this PR is to avoid setter/getter method name conflicts,
not only for setAttributes().
See https://github.com/codeigniter4/CodeIgniter4/pull/7230/files#diff-b43fcc536c7ffa370ea5d043988d111223780b2dade0198d7b91818f9bd7ab62

@MGatner MGatner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look good.

@iRedds

iRedds commented Feb 21, 2023

Copy link
Copy Markdown
Collaborator

As far as I remember, PSR does not recommend using the underscore character as a prefix for method names.
I have already expressed my opinion about the names of methods for accessing attributes more than once.
get<name>Attribute

@kenjis

kenjis commented Feb 22, 2023

Copy link
Copy Markdown
Member Author

We don't remove get<name>() getter.
So get<name>Attribute() may conflict to another column.

If there are columns parent and parent_attribute and
getParent() and getParentAttribute(),
getParentAttribute() is used when getting parent.
This is a breaking change.

@kenjis
kenjis merged commit b28b6ae into codeigniter4:4.4 Feb 27, 2023
@kenjis
kenjis deleted the feat-entity-new-setter branch February 27, 2023 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement PRs that improve existing functionalities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants