Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Add Naming strategy for Hydrators #5364

Merged
merged 3 commits into from
Jan 3, 2014

Conversation

ronan-gloo
Copy link
Contributor

This commit is a purpose for key / properties transformations supports through externalized strategies.
It introduce the same behavior implemented for values, but on names level.

  • keys / properties translations during hydration / extraction doesn't need to be hardcoded anymore in hydrators.
  • Components are reusable through many hydrators.
  • Despite values strategies, Only one "global" naming strategy can be defined by Hydrator.
  • As side effect, non wildcard values strategies will be dependent of the naming strategy hydrate output

An Underscore <-> CamelCase strategy is shipped by default, in order to keep ClassMethods underscore feature alive, and keep the BC. (the strategy is set / unset on setUnderscoreSeparatedKeys)

For others Hydrators, you'll need to set the naming strategy manually:

$hydrator = new Zend\Stdlib\Hydrator\ObjectProperty;
$hydrator->setNamingStrategy(new Zend\Stdlib\Hydrator\NamingStrategy\UnderscoreNamingStrategy);

Let me know if you're interested to merge this feature, i'll push unit tests & doc.

ronan-gloo added 2 commits October 27, 2013 16:08
This commit introduce strategies for key / properties transformations
@@ -25,6 +29,13 @@
protected $strategies;

/**
* The list of naming strategies that this hydrator has.
Copy link
Member

Choose a reason for hiding this comment

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

If this is a list, how can it be of type NamingStrategyInterface? Shouldn't it be an array or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't updated the doc-block in the last commit, my bad.

@DASPRiD
Copy link
Member

DASPRiD commented Oct 27, 2013

I like the idea in general, pretty useful in many cases.

*
* @param NamingStrategyInterface $strategy The naming to register.
*
* @return NamingStrategyEnabledInterface
Copy link
Member

Choose a reason for hiding this comment

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

@return self, and no empty-line between params and return annotations.

@weierophinney
Copy link
Member

Overall, 👍

One concern I have is that hydrators then become stateful, as they have specific naming strategies composed. What I cannot recall, however, is if the HydratorManager returns a new instance each time, or the same instance; if the former, we're fine.

Also, it might be nice to have a way to pass the naming strategy when retrieving a Hydrator from the HydratorManager; plugin managers already allow passing an array of options when you call get(), and it could be used for this.

@ronan-gloo
Copy link
Contributor Author

The Hydrator Manager return a new instance on each time be default. But you're not required to use a naming strategy, so the hydrators are still stateless from this point of view.

The idea of passing naming strategy sounds interesting, but what about filters and classic strategies ?

@weierophinney
Copy link
Member

@ronan-gloo Ideally we should also allow passing filters and classic strategies; this would be a nice start, however. Let me know how you want to proceed.

@bakura10
Copy link
Contributor

That's for this specific thing that I'd thought of Data transformers.

I'm afraid that the hydrator is getting too much features now :/. As a lot of other components may need that kind of transformation (input filters for instance), that's why I think it would be better to abstract that kind of thing "somewhere else".

@ronan-gloo
Copy link
Contributor Author

@weierophinney That's right, as long as the interfaces are similar, it will be more flexible (but maybe a bit confusing for the user ?) to specify the "classic" StrategyInterface for naming strategy.I've separated interfaces in order to make the idea clearer.
The FilterInterface do not specify if we are in extraction or hydration context, that's why it seems to be limited as long as we don't explicitly contraints extract / hydrate filters additions in Hydration classe. Strategy interface seem's much straight forward solution to supports this feature.

@bakura10
Your general workflow suggestion for data is very interesting. Correct me if i'm wrong, but the issue for Data transformers seem's for the nest Zf release. How are your plans to integrate it for ZF2 ?

@bakura10
Copy link
Contributor

@ronan-gloo , I currently have no plans integrating it into ZF2. It's just an idea though...

$this->underscoreSeparatedKeys = $underscoreSeparatedKeys;
$this->underscoreSeparatedKeys = (bool) $underscoreSeparatedKeys;

if (true === $this->underscoreSeparatedKeys) {
Copy link
Member

Choose a reason for hiding this comment

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

Since we know this is a boolean at this point (you cast it to bool in line 82), you can get rid of the true === bit.

@ghost ghost assigned weierophinney Jan 3, 2014
weierophinney added a commit that referenced this pull request Jan 3, 2014
Add Naming strategy for Hydrators
weierophinney added a commit that referenced this pull request Jan 3, 2014
- we know we have a boolean; remove true === test
- combine if statement from body with else parent
weierophinney added a commit that referenced this pull request Jan 3, 2014
- Modified class to use existing filters, via a filter chain, to perform
  the work. Filter chains are stored as static properties as the
  implementation will not change between instances.
weierophinney added a commit that referenced this pull request Jan 3, 2014
@weierophinney weierophinney merged commit 2810655 into zendframework:develop Jan 3, 2014
weierophinney added a commit that referenced this pull request Jan 3, 2014
weierophinney added a commit that referenced this pull request Jan 3, 2014
- Updated copyright year
- Removed unneeded annotations
- Removed empty lines at start and finish of classes
weierophinney added a commit that referenced this pull request Jan 3, 2014
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
…evelop

Add Naming strategy for Hydrators
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
- we know we have a boolean; remove true === test
- combine if statement from body with else parent
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
- Modified class to use existing filters, via a filter chain, to perform
  the work. Filter chains are stored as static properties as the
  implementation will not change between instances.
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
- Updated copyright year
- Removed unneeded annotations
- Removed empty lines at start and finish of classes
weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants