Skip to content

Add get/set support for DIContainerTrait::setDefaults()#245

Closed
mvorisek wants to merge 4 commits intodevelopfrom
set_defaults_with_setXxx
Closed

Add get/set support for DIContainerTrait::setDefaults()#245
mvorisek wants to merge 4 commits intodevelopfrom
set_defaults_with_setXxx

Conversation

@mvorisek
Copy link
Copy Markdown
Member

@mvorisek mvorisek commented Jun 12, 2020 •
edited
Loading

fixes #206 , needed for #227

done, but need tests

maybe even add/remove with syntax like class.adds = ['x', 'y'], ...

@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch 2 times, most recently from 236fe52 to aae0b02 Compare June 12, 2020 23:06
@mvorisek mvorisek requested a review from DarkSide666 June 12, 2020 23:13
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch 2 times, most recently from fc1ba36 to fc0e208 Compare June 13, 2020 12:38
@atk4 atk4 deleted a comment from codecov bot Jun 14, 2020
@DarkSide666
Copy link
Copy Markdown
Member

DarkSide666 commented Jun 30, 2020

Not sure about this.
@abbadon1334 comment #206 (comment) sounds quite legit.

@georgehristov
Copy link
Copy Markdown
Collaborator

georgehristov commented Jun 30, 2020

I am "for" it and was even proposing it in the chat some time ago.

@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch 2 times, most recently from e1aa2c9 to ada20a4 Compare July 8, 2020 09:04
@mvorisek mvorisek marked this pull request as draft August 25, 2020 08:57
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from ada20a4 to 5bf9d5d Compare August 26, 2020 10:51
@mvorisek mvorisek requested a review from georgehristov August 26, 2020 11:19
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from 33e9087 to 79a7963 Compare August 26, 2020 11:22
@mvorisek
Copy link
Copy Markdown
Member Author

mvorisek commented Aug 26, 2020 •
edited
Loading

Not sure about this.
@abbadon1334 comment #206 (comment) sounds quite legit.

we can implement it using 2pass approach, ie.

  1. set DIRECTLY ALL values FIRST (if declared or detected to be magical)
  2. then for each value (if setter exists):
    a) reset to old value
    b) set using invoking setXxx()

@abbadon1334 does this address your comment?

@DarkSide666 @georgehristov wdyt, any better idea?

@mvorisek
Copy link
Copy Markdown
Member Author

mvorisek commented Aug 26, 2020 •
edited
Loading

another option might be to use property anotations for getter/setter method names, but looks like quite a lot horsepower for the current state of this project now

@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from f652977 to 1b890e2 Compare August 26, 2020 19:01
mvorisek
mvorisek commented Aug 27, 2020
View reviewed changes
src/DiContainerTrait.php Outdated
$isMissing = true;

try {
$origValue = $this->{$name} ?? null;
Copy link
Copy Markdown
Member Author

@mvorisek mvorisek Aug 27, 2020

Choose a reason for hiding this comment

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

this will still not work if null...

@mvorisek mvorisek mentioned this pull request Dec 27, 2020
Closed
@mvorisek mvorisek mentioned this pull request Jul 26, 2021
Closed
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from 1b890e2 to d319364 Compare October 4, 2021 11:41
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from d319364 to d4304aa Compare June 13, 2023 15:37
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from d4304aa to e85b785 Compare June 13, 2023 15:42
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from e1b8e2e to 7cd9e76 Compare July 16, 2023 14:51
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch 2 times, most recently from a3e0eec to 7ac2e7e Compare October 1, 2023 13:46
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from 7ac2e7e to 739bb7a Compare March 31, 2024 21:51
@mvorisek mvorisek force-pushed the set_defaults_with_setXxx branch from 739bb7a to b4998f9 Compare March 31, 2024 22:06
@mvorisek
Copy link
Copy Markdown
Member Author

mvorisek commented Mar 31, 2024

closing in favor of https://wiki.php.net/rfc/property-hooks

@mvorisek mvorisek closed this Mar 31, 2024
@mvorisek mvorisek deleted the set_defaults_with_setXxx branch March 31, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DarkSide666 DarkSide666 Awaiting requested review from DarkSide666

@abbadon1334 abbadon1334 Awaiting requested review from abbadon1334

@romaninsh romaninsh Awaiting requested review from romaninsh

@georgehristov georgehristov Awaiting requested review from georgehristov

Assignees

No one assigned

Labels

MAJOR

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow/prioritize setter methods in DIContainerTrait::setDefaults()

3 participants

@mvorisek @DarkSide666 @georgehristov