simplifier les instructions if imbriquées
J'implémente une fonctionnalité de recherche et en fonction du paramètre de requête, j'utilise une classe différente pour rechercher.
class Search {
public function getResults()
{
if (request('type') == 'thread') {
$results = app(SearchThreads::class)->query(); } elseif (request('type') == 'profile_post') { $results = app(SearchProfilePosts::class)->query();
} elseif (request()->missing('type')) {
$results = app(SearchAllPosts::class)->query();
}
}
Maintenant, lorsque je veux rechercher des fils, j'ai le code suivant.
class SearchThreads{
public function query()
{
$searchQuery = request('q');
$onlyTitle = request()->boolean('only_title'); if (isset($searchQuery)) {
if ($onlyTitle) { $query = Thread::search($searchQuery); } else { $query = Threads::search($searchQuery); } } else { if ($onlyTitle) {
$query = Activity::ofThreads(); } else { $query = Activity::ofThreadsAndReplies();
}
}
}
}
Pour expliquer le code.
Si l'utilisateur entre un mot de recherche ( $ searchQuery ), utilisez Algolia pour rechercher, sinon effectuez une requête directement dans la base de données.
Si l'utilisateur entre un mot de recherche
- Utilisez l' index des threads si l'utilisateur a coché la case onlyTitle
- Utilisez l' index Threads si l'utilisateur n'a pas coché la case onlyTitle
Si l'utilisateur n'entre pas un mot de recherche
- Obtenez tous les threads si l'utilisateur a coché la case onlyTitle
- Obtenez tous les fils de discussion et les réponses si l'utilisateur n'a pas coché la case onlyTitle
Existe-t-il un modèle pour simplifier les instructions if imbriquées ou devrais-je simplement créer une classe séparée pour les cas où
- un utilisateur a saisi un mot de recherche
- un utilisateur n'a pas entré un mot de recherche
Et à l'intérieur de chacune de ces classes pour vérifier si l'utilisateur a coché la case onlyTitle
Réponses
Je refactoriserais ce code en ceci:
Laissez le paramètre request pour unifier les méthodes de recherche dans une interface.
interface SearchInterface
{
public function search(\Illuminate\Http\Request $request); } class Search { protected $strategy;
public function __construct($search) { $this->strategy = $search; } public function getResults(\Illuminate\Http\Request $request)
{
return $this->strategy->search($request);
}
}
class SearchFactory
{
private \Illuminate\Contracts\Container\Container $container; public function __construct(\Illuminate\Contracts\Container\Container $container)
{
$this->container = $container;
}
public function algoliaFromRequest(\Illuminate\Http\Request $request): Search { $type = $request['type']; $onlyTitle = $request->boolean('only_title'); if ($type === 'thread' && !$onlyTitle) { return $this->container->get(Threads::class);
}
if ($type === 'profile_post' && !$onlyTitle) {
return $this->container->get(ProfilePosts::class); } if (empty($type) && !$onlyTitle) { return $this->container->get(AllPosts::class);
}
if ($onlyTitle) { return $this->container->get(Thread::class);
}
throw new UnexpectedValueException();
}
public function fromRequest(\Illuminate\Http\Request $request): Search { if ($request->missing('q')) {
return $this->databaseFromRequest($request);
}
return $this->algoliaFromRequest($request);
}
public function databaseFromRequest(\Illuminate\Http\Request $request): Search { $type = $request['type']; $onlyTitle = $request->boolean('only_title'); if ($type === 'thread' && !$onlyTitle) { return $this->container->get(DatabaseSearchThreads::class);
}
if ($type === 'profile_post' && !$onlyTitle) {
return $this->container->get(DatabaseSearchProfilePosts::class); } if ($type === 'thread' && $onlyTitle) { return $this->container->get(DatabaseSearchThread::class);
}
if ($request->missing('type')) { return $this->container->get(DatabaseSearchAllPosts::class);
}
throw new InvalidArgumentException();
}
}
final class SearchController
{
private SearchFactory $factory; public function __construct(SearchFactory $factory)
{
$this->factory = $factory;
}
public function listResults(\Illuminate\Http\Request $request) { return $this->factory->fromRequest($request)->getResults($request);
}
}
La chose à retenir est qu'il est très important de ne pas impliquer la demande dans les constructeurs. De cette façon, vous pouvez créer des instances sans demande dans le cycle de vie de l'application. C'est bon pour la mise en cache, la testabilité et la modularité. Je n'aime pas non plus l'application et les méthodes de demande car elles extraient les variables de rien, réduisant la testabilité et les performances.
class Search
{
public function __construct(){
$this->strategy = app(SearchFactory::class)->create(); } public function getResults() { return $this->strategy->search();
}
}
class SearchFactory
{
public function create()
{
if (request()->missing('q')) {
return app(DatabaseSearch::class);
} else {
return app(AlgoliaSearch::class);
}
}
}
class AlgoliaSearch implements SearchInterface
{
public function __construct()
{
$this->strategy = app(AlgoliaSearchFactory::class)->create(); } public function search() { $this->strategy->search();
}
}
class AlgoliaSearchFactory
{
public function create()
{
if (request('type') == 'thread') {
return app(Threads::class);
} elseif (request('type') == 'profile_post') {
return app(ProfilePosts::class);
} elseif (request()->missing('type')) {
return app(AllPosts::class);
} elseif (request()->boolean('only_title')) {
return app(Thread::class);
}
}
}
Où les classes créées dans AlgoliaSearchFactory sont des agrégateurs Algolia, la méthode de recherche peut donc être appelée sur n'importe laquelle de ces classes.
Est-ce que quelque chose comme ça le rendrait plus propre ou même pire?
En ce moment, j'ai des stratégies qui ont des stratégies qui me semblent trop.
J'ai essayé de mettre en œuvre une bonne solution pour vous, mais j'ai dû faire des hypothèses sur le code.
J'ai découplé la demande de la logique du constructeur et ai donné à l'interface de recherche un paramètre de demande. Cela rend l'intention plus claire que de simplement extraire la demande de rien avec la fonction de demande.
final class SearchFactory
{
private ContainerInterface $container; /** * I am not a big fan of using the container to locate the dependencies. * If possible I would implement the construction logic inside the methods. * The only object you would then pass into the constructor are basic building blocks, * independent from the HTTP request (e.g. PDO, AlgoliaClient etc.) */ public function __construct(ContainerInterface $container)
{
$this->container = $container;
}
private function databaseSearch(): DatabaseSearch
{
return // databaseSearch construction logic
}
public function thread(): AlgoliaSearch
{
return // thread construction logic
}
public function threads(): AlgoliaSearch
{
return // threads construction logic
}
public function profilePost(): AlgoliaSearch
{
return // thread construction logic
}
public function onlyTitle(): AlgoliaSearch
{
return // thread construction logic
}
public function fromRequest(Request $request): SearchInterface { if ($request->missing('q')) {
return $this->databaseSearch(); } // Fancy solution to reduce if statements in exchange for legibility :) // Note: this is only a viable solution if you have done correct http validation IMO $camelCaseType = Str::camel($request->get('type')); if (!method_exists($this, $camelCaseType)) { // Throw a relevent error here } return $this->$camelCaseType(); } } // According to the code you provided, algoliasearch seems an unnecessary wrapper class, which receives a search interface, just to call another search interface. If this is the only reason for its existence, I would remove it final class AlgoliaSearch implements SearchInterface { private SearchInterface $search;
public function __construct(SearchInterface $search) { $this->search = $search; } public function search(Request $request): SearchInterface {
return $this->search->search($request);
}
}
Je ne suis pas non plus sûr de l'intérêt de la classe Search. S'il ne renomme efficacement les méthodes de recherche que pour getResults, je ne suis pas sûr de l'intérêt. C'est pourquoi je l'ai omis.
J'ai dû écrire tout cela pour rendre le problème compréhensible.
Le SearchFactory prend tous les paramètres requis et en fonction de ces paramètres, il appelle soit AlgoliaSearchFactory ou DatabaseSearchFactory pour produire l'objet final qui sera retourné.
class SearchFactory
{
protected $type; protected $searchQuery;
protected $onlyTitle; protected $algoliaSearchFactory;
protected $databaseSearchFactory; public function __construct( $type,
$searchQuery, $onlyTitle,
DatabaseSearchFactory $databaseSearchFactory, AlgoliaSearchFactory $algoliaSearchFactory
) {
$this->type = $type;
$this->searchQuery = $searchQuery;
$this->onlyTitle = $onlyTitle;
$this->databaseSearchFactory = $databaseSearchFactory;
$this->algoliaSearchFactory = $algoliaSearchFactory;
}
public function create()
{
if (isset($this->searchQuery)) { return $this->algoliaSearchFactory->create($this->type, $this->onlyTitle);
} else {
return $this->databaseSearchFactory->create($this->type, $this->onlyTitle);
}
}
}
La DatabaseSearchFactory basée sur le $ type et les paramètres onlyTitle qui sont passés à partir de SearchFactory retourne un objet qui est l'objet final qui doit être utilisé pour obtenir les résultats.
class DatabaseSearchFactory
{
public function create($type, $onlyTitle)
{
if ($type == 'thread' && !$onlyTitle) {
return app(DatabaseSearchThreads::class);
} elseif ($type == 'profile_post' && !$onlyTitle) {
return app(DatabaseSearchProfilePosts::class);
} elseif ($type == 'thread' && $onlyTitle) {
return app(DatabaseSearchThread::class);
} elseif (is_null($type)) {
return app(DatabaseSearchAllPosts::class);
}
}
}
Même logique avec DatabaseSearchFactory
class AlgoliaSearchFactory
{
public function create($type, $onlyTitle) { if ($type == 'thread' && !$onlyTitle) { return app(Threads::class); } elseif ($type == 'profile_post' && !$onlyTitle) { return app(ProfilePosts::class); } elseif (empty($type) && !$onlyTitle) { return app(AllPosts::class); } elseif ($onlyTitle) {
return app(Thread::class);
}
}
}
Les objets créés par AlgoliaSearchFactory ont une méthode de recherche qui nécessite une valeur $ searchQuery
interface AlgoliaSearchInterface
{
public function search($searchQuery);
}
Les objets créés par DatabaseSearchFactory ont une méthode de recherche qui ne nécessite aucun paramètre.
interface DatabaseSearchInterface
{
public function search();
}
La classe Search prend maintenant comme paramètre l'objet final qui est produit par SearchFactory qui peut implémenter AlgoliaSearchInterface ou DatabaseSearchInterface c'est pourquoi je n'ai pas tapé d'indication
La méthode getResults doit maintenant trouver le type de la variable de recherche (quelle interface elle implémente) afin de passer le $ searchQuery comme paramètre ou non.
Et c'est ainsi qu'un contrôleur peut utiliser la classe Search pour obtenir les résultats. Recherche de classe {stratégie $ protégée;
public function __construct($search) { $this->strategy = $search; } public function getResults() { if(isset(request('q'))) { $results = $this->strategy->search(request('q')); } else { $results = $this->strategy->search(); } } } class SearchController(Search $search)
{
$results = $search->getResults();
}
Selon toutes les suggestions @Transitive, c'est ce que j'ai trouvé. La seule chose que je ne peux pas résoudre est comment appeler la recherche dans la méthode getResults sans avoir une instruction if.