Circular dependencies makes extensibility difficult with scopes and transformers

[brianmuse]

We use fractal pretty religiously throughout our REST API and recently have had need to extend its capabilities a bit so that we can more efficiently load our entities as we're traversing the scope tree.

That's a separate topic, but what it's brought to light is a lot of difficulty in extending the library due to circular dependencies and muddled responsibilities between the scope and the transformer classes, as well as some private methods that would be very useful if protected.

I'll try my best to summarize discoveries and make suggestions:

Scope would be better as an interface

With the introduction of the ScopeFactory, this would allow us to implement our own scopes more easily without having to extend and adapt to the base scope.

Scope toArray mixes responsibility with transformer private methods

The toArray method in the scope is a recursive mechanism that transforms the resource into an array, then merges in the necessary "included" resources by calling toArray on those child scopes.

The issue arises in that it calls processIncludedResources(Scope $scope, $data) on the transformer.

This is immediately a circular dependency, as the scope depends on the transformer and the transformer depends on the current scope. This is more illustrated by the call chain that occurs within this method:

public function processIncludedResources(Scope $scope, $data) {
  // call figureOutWhichIncludes
  // call includeResourceIfAvailable foreach include
}

// Private which prevents `processIncludedResources` from being overloadable unless you 
// reimplement (copy paste) this entire method in your child class.
private function figureOutWhichIncludes(Scope $scope) {
  // ...
}

// Private which prevents `processIncludedResources` from being overloadable unless you 
// reimplement (copy paste) this entire method in your child class.
private function includeResourceIfAvailable(
        Scope $scope,
        $data,
        $includedData,
        $include
    ) {
  // Here is the main issue... From this method we call:
  $scope->toArray();
  // Now we've completed the full circle.
}

// If this was public it could be called from the scope instead of by
// private methods in the transformer classes. The scope is already responsible for 
// calling the `transform` method, so this would make sense.
protected function callIncludeMethod(Scope $scope, $includeName, $data) {
  // ...
}

Where I expect that the responsibility of the Scope is to call all the necessary include and transform methods on the transformer and then compose those into the array as part of toArray, instead there is a complex call chain the mixes these responsibilities between the two classes.

Recommend that the TransformerAbstract really be a stupid object with the include and transform methods and let the Scope object (or some new higher level of abstraction) compose the data. So essentially move a lot of the logic from the transformer into the scope class to remove the circular dependency and make it easier (and possible) to modify the logic of traversing the fractals when creating our own scope implementations.

---

Let me know if anything is unclear here and I'll be happy to try to clarify a bit better.

[fesor]

I removed this circular dependency in my fork. Since processIncludedResources and callIncludeMethod is marked as @internal I don't even think that this is BC break.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 4 weeks if no further activity occurs. Thank you for your contributions.