fuel / helpers

FuelPHP Framework - Generic helpers library
MIT License
17 stars 7 forks source link

Weird DataContainer parent implementation #5

Closed sagikazarmark closed 10 years ago

sagikazarmark commented 10 years ago

It's a bit weird that in case of parent enabled getContents() returns the parent data as well. It should be a reverse of setContents. IMO it is rather annoying than helpful (in most cases).

It could be controlled by a parameter, etc.

WanWizard commented 10 years ago

The idea behind is that a data container (as viewed from the outside) is a single data collection, but internally can be made up of multiple collections, with inheritance. The behavior of this inheritance can be controlled through methods enableParent/disableParent.

The usage of this becomes obvious when you look at stuff like the v2 equivalents off config, input and lang, where this mechanism is used extensively.

It is also used in when processing requests. They should inherit all "environment" information from the parent request (or the server environment on the main request), but must not be allowed to alter it (which would destroy the decoupled way they must operate).

Also, by default a container doesn't have a parent, so you can control this behaviour yourself.

Any specific use-case where this behaviour might be an issue?

sagikazarmark commented 10 years ago

I accept this, but I still think that there should be an option for retrieving data from one container without parent data. This type of relationship does not always mean inheritance. (Eg. some sort of nested dataset) It is also a general container implementation, not just part of the framework, the functionality you mentioned should be optional.

WanWizard commented 10 years ago

In all uses for which the DataContainer was designed, inheritance is required, it's why it is designed this way. And all implementations inside the framework require this.

If you need a generic DataContainer for a different purpose, you can control yourself whether or not it has a parent, and if it has, whether or not it is used. So the option you ask for is already there.

I accept it can be used stand alone, but we're building a framework first here, and separate packages second. I do not accept making components "overly" generic when it has impact on the requirements of the framework.