Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add source, destination and property stacks in context array #44

Merged
merged 1 commit into from
Aug 18, 2019

Conversation

Toilal
Copy link
Contributor

@Toilal Toilal commented Aug 8, 2019

This brings additionnal data into context, source stack, destination stack and property stack.

Those context data could be used in custom operation or custom mappers provide more context of the object graph that is currently converted.

It could also help to implement some feature requests like #36, #16, #14 ...

@Toilal Toilal changed the title Add DESTINATION_STACK and PROPERTY_STACK in context array Add source, destination and property stacks in context array Aug 8, 2019
README.md Outdated Show resolved Hide resolved
private function push($key, $value, &$context)
{
if (!array_key_exists($key, $context)) {
$stack = [];
Copy link
Owner

Choose a reason for hiding this comment

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

How do you feel about using an actual stack (e.g. SplStack) instead of an array? I think using SplStack makes more sense for what we're doing here (pushing/popping). But for the end user an array would be more convenient when they are doing something with the context (e.g. all the array_ functions are available to them etc.). So I guess we'd best stick to an array :)

Copy link
Contributor Author

@Toilal Toilal Aug 11, 2019

Choose a reason for hiding this comment

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

I think we should stick on array too. Maybe I could extract push/pop private methods into a utility class though (in the middlareware pull request, those are duplicated)

@mark-gerarts
Copy link
Owner

I'm merging this, but I'm gonna wait with tagging a new release until I've been through all the PR's. So this is gonna sit on master for a while.

Anyways, thanks for your contribution!

@mark-gerarts mark-gerarts merged commit a0a7d7a into mark-gerarts:master Aug 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants