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

Segregation HydratorInterface #4908

Closed
wants to merge 2 commits into from
Closed

Segregation HydratorInterface #4908

wants to merge 2 commits into from

Conversation

spalax
Copy link
Contributor

@spalax spalax commented Jul 30, 2013

HydratorInterface contains two responsibilities it is Hydration and Extraction, my suggestion is to segregate this responsibilities at least with Interfaces. Some times you need only extract from objects who implement both hydrate and extract.

…, my suggestion is to segregate this responsibilities at least with Interfaces.
@Ocramius
Copy link
Member

I really really really like this :) 👍

It obviously calls for some more from the hydration component, but I guess that can't really live before zf3

@spalax
Copy link
Contributor Author

spalax commented Jul 30, 2013

It is would super great if this fix will be merged to the zf2. This fix will not harm zf2 , but it is will provide abilities to write more clear code.

@Ocramius
Copy link
Member

@spalax I'm talking about revising the hydrator interface, which is obviously out of discussion for 2.x

@weierophinney
Copy link
Member

I'd go a step further:

  • Rename "ExtractorInterface" to "ExtractionInterface"
  • Create a "HydrationInterface"
  • Modify HydratorInterface to extend both of the above

This will allow us to keep BC, while having fully segregated interfaces.

@Ocramius
Copy link
Member

@weierophinney doesn't ExtractionInterface sound a bit weird? :| Otherwise, yes, the idea is good

@spalax
Copy link
Contributor Author

spalax commented Aug 19, 2013

@weierophinney agree. Will done soon.

@spalax
Copy link
Contributor Author

spalax commented Aug 19, 2013

@weierophinney I think in future it is should not be Hydrator who implements Extraction and Hydration but Hydrator and Extractor as independent objects, and Abstract Manager (kind of Facade) who will be able to contains aggregation of Extractors and Hydrators and will implement interface with methods extract and hydrate. Does it make sense ?

@Ocramius
Copy link
Member

@bakura10 ping - you must see this

@weierophinney
Copy link
Member

I think in future it is should not be Hydrator who implements Extraction and Hydration but Hydrator and Extractor as independent objects, and Abstract Manager (kind of Facade) who will be able to contains aggregation of Extractors and Hydrators and will implement interface with methods extract and hydrate.

Sounds good -- let's get these interfaces in first, and we can look at that for a later release and/or 3.0.

Thanks again, @spalax :)

@bakura10
Copy link
Contributor

Good idea, though it's a bit strange to have HydratorInterface implements HydrationInterface and ExtractionInterface, so I kinda like the idea of having two distinct interfaces. But we need a good name for having an interface that aggregate both. Maybe ConverterInterface, dunno.

@Ocramius , I'll soon re-work on my hydrator refactor for ZF3, will try to get all those suggestions into account.

@Ocramius
Copy link
Member

I'll ask around. Don't have better ideas either.

@weierophinney
Copy link
Member

@bakura10 and @Ocramius We have to keep the current naming (i.e., HydratorInterface) and its current scope of functionality (i.e., it assumes both responsibilities, extraction and hydration) for the lifecycle of 2.x to keep things backwards compatible. It may be odd, but it's what we have.

weierophinney added a commit that referenced this pull request Aug 20, 2013
weierophinney added a commit that referenced this pull request Aug 20, 2013
@ghost ghost assigned weierophinney Aug 20, 2013
@weierophinney
Copy link
Member

Merged to develop for release with 2.3.0.

@Ocramius
Copy link
Member

@weierophinney wasn't doubting that for 2.x the interface wouldn't change - we're just eager to experiment new approaches for 3.x

weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
…toring_extract_interface

Segregation HydratorInterface
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