Level 61
I would always use Dependency Injection without manual binding in the container. So the first option for me is the only option.
Oct 13, 2020
13
Level 13
instantiation of chained filters
I have different Filter classes and each class has a number of filter methods. There is ( so far ) one case where I need to apply filters from 2 classes. What I did is that I created another class named FilterManager which stores a list of all the Filter classes you need and then it iterates over the list and applies the filters.
class ThreadFilters
{
protected $builder;
protected $supportedFilters = ['newThreads'];
public function newThreads($userId)
{
$this->builder->orderBy('created_at', 'DESC');
}
}
class FilterManager
{
protected $filters = [];
public function addFilter($filter)
{
$this->filters[] = $filter;
}
public function apply($builder)
{
foreach($this->filters as $filter)
{
$filter->builder = $builder;
foreach($filter->supportedFilters as $filterMethod)
{
$filter->$filterMethod()
}
$builder = $filter->builder;
}
}
}
Now in the controller I can do something like
First Approach
class ThreadController(FilterManager $filterManager)
{
$filters = $filterManager->addFilter(new ThreadFilter());
$filters->apply($someBuilder);
}
On the other hand I could use the app function
app(ThreadFilters::class)
and in the service container basically I could do
$this->app->bind(ThreadFilters::class, function($app){
$filterManager = new FilterManager();
$filterManager->addFilter(new ThreadFilters());
return $filterManager;
});
and therefore my controller would look like
Second Approach
class ThreadController()
{
$filters = app(ThreadFilters::class);
$filter->apply($someBuilder);
}
My question is which solution is good/acceptable.
In the First Approach the FilterManager dependency is injected and the controller has to add the ThreadFilters
In the Second Approach no dependencies are injected and the controller has to use the service container to hide the specifics and make the controller has less knowledge about the filters.
Level 13
@bugsysha thanks. Isn't an issue that i have to manually
$filterManager->addFilter(new ThreadFilters());
where ThreadFilters i guess is also a dependency that is hidden.
or it is even better to do the following
class ThreadController(FilterManager $filterManager, ThreadFilters $threadFilters)
{
$filters = $filterManager->addFilter($threadFilters);
$filters->apply($someBuilder);
}
Level 61
I'll name few reasons why in my mind it is better to do the first option:
- it keeps the logic in sight, new developers especially juniors might not know it can be done in the service provider
- unit testing, it can be tested without booting the whole framework
- service provider class should not have to change if you add another filter
Now just to be clear, I'm not saying I would do it the way you showed in the first option, but it is far closer.
1 like
Level 13
@bugsysha thanks again! any suggestions for a better approach are appreciated :)
Level 61
Why are you adding filters at runtime in the controller?
Level 61
Sorry, maybe the question is not clear enough. Why do you have an option to add filters in your FilterManager class?
Level 13
@bugsysha it's not quite clear to me what do you mean by "why".
In the ThreadController and more specifically in the method index, i know that i need the ThreadFilters, that's why i add the filter ThreadFilters. In other places i might need a different filter. And in some cases i need two different Filter classes
class ThreadController extends Controller
{
public function index(FilterManager $filterManager)
{
$filterManager->addFilter(new ThreadFilters());
// here i know that i need the ThreadFilters
}
}
Level 61
Are there any default filters in the FilterManager?
Level 13
@bugsysha oh now i understand your question.
Because in some cases i might need to add 2 different Filter classes
for example
$filterManager->addFilter(new ThreadFilters(());
$filterManager->addFilter(new PostFilters());
Then when i call
$filterManager->apply($someBuilder);
I apply the filters from both Filter classes. In addition to that, in some cases these Filter classes have some methods in common that are inherited from another Filter class and in the FilterManager i prevent applying the same filter twice ( which i don't know if it makes much sense since the filters are actually SQL where statements which i guess are not costly )
Level 13
@bugsysha no there are no default filters in FilterManager
The FilterManager exists to find the requested filters that are passed from an HTTP request ( the requested filters correspond to methods in Filter classes ) . I then manually chain the required Filter classes depending on where i need the FilterManager and finally i apply the filters using the FilterManager
Level 61
Then put all that managing logic inside FilterManager.
Problem that you have now is following:
- if you want to use the same set of filters you would have to duplicate your code
- someone who didn't work on this feature would have to search through the codebase to see how it is being done
- there is no business logic showing which filters should be used together cause you might have some that should not be mixed
- and many more
Put everything inside FilterManager class.
class ThreadController extends Controller
{
public function index(FilterManager $filterManager)
{
$filterManager->withThreadFilters()->apply($builder);
}
}
class FilterManager
{
// ... your code
public function withThreadFilters(): self
{
$this->filters = []; // reset the filters array so it is not mixed with some other set of filters
$this->filters[] = new ThreadFilter;
$this->filters[] = new PostFilter;
// ... and so on
return $this;
}
}
This way it is clear which filters go together and which purpose they serve. You can remove addFilter() method and simplification is always good. There are many more options you can use here, but this is as simple as it gets.
1 like
Level 13
@bugsysha that cleaner indeed.
initially what i wanted to do is something like
public function index(ThreadFilters $filters)
{
$filters->apply($someBuilder);
}
So the controller wouldn't have to know anything except for applying the filters.
But unfortunately since i had to chain Filter in some cases i created the FilterManager and then i couldn't use the aforementioned approach.
However, your suggestions is much better than my final approach. so thank you again
Level 61
You are very welcome. Thanks for the Best Reply award.
1 like
Please or to participate in this conversation.