Be part of JetBrains PHPverse 2026 on June 9 – a free online event bringing PHP devs worldwide together.

vincej's avatar
Level 15

Are 50+ Controllers a sign of code smell ?

I have a controller for each topic ( eg products, suppliers etc ) and so far I can see 250 pages. Each controller has been built with Jeffrey's Generator, so each topic will have the standard CRUD.

I come from a CodeIgniter world, and I want to do things the correct "Laravel Way".

I am nervous that this approach might be less than "Laravel" and more Codeigniter.

What do you think ?

Thanks !

0 likes
6 replies
JarekTkaczyk's avatar

code smell refers rather to design issues, like tight coupling etc. Make your code SOLID, don't worry about number of files/classes as long as they all have their reponsibility and you DRY.

Also you may read http://symfony.com/doc/current/book/index.html regarding controllers - you will find there, repeated at least a few times, emphasized that sole responsibility of a controller (symfony refers to a method, not a controller class, but it doesn't matter) is to take the request, call other services to get the job done, and finally return the response.

That's why controller should be skinny and shouldn't do pretty much anything apart from handling request and returning response.

That said, there's nothing wrong with having many controllers if your HTTP layer requires this.

1 like
bestmomo's avatar

If each resource has its controller it's just fine I think, regardless of the number. But maybe you could pool some code. I've had an application with many topics and I made a BaseResourceController with all casual methods.

1 like
vincej's avatar
Level 15

Thanks - that make me feel better. Again following Jeffrey's lessons I am using resourceful routing ( it's great ! ) and so my routes are marrying nicely into my controllers.

@bestmomo I don't really understand what you mean by a "BaseResourceController" ... can you give me a hint ?

Thanks !

bestmomo's avatar

@vincej Just a bit of code. If you have a BaseResourceController with some methods like "create" :

abstract class BaseResourceController extends \Illuminate\Routing\Controller {

    protected $gestion;
    protected $base;

    public function create()
    {
        return View::make($this->base.'.vues.create',  $this->gestion->create());
    }
}

You can have a LivreController with this code :

class LivreController extends BaseResourceController {

    public function __construct(LivreGestion $gestion)
    {
            $this->gestion = $gestion;
            $this->base = class_basename(__NAMESPACE__);
    }

}

And the same for all methods. And "gestion" is the repository.

It's only an example and not necessarily suitable for you.

nolros's avatar

@bestmomo very cool, this would work really well for many of the getForm type requests. Only issue and I'm stretching is readability, in that some of the controller code gets abstraction, but no more than any other abtsract class model, repo, etc. Very nice!

pmall's avatar

Each topic has basic crud functionality ?? You can abstract this away. I made this for an app that have ~10 entities.

First, you need a topic interface.

<?php namespace App\Types;

interface TopicInterface{
  public function performSave(Array $data = []);
  public function performDestroy(Array $data = []);
  // Whatever method common to all topics
}

Then each topic implements it

<?php namespace App\Entities;

class Supplier extends Model implements TopicInterface{

  // Implementation of performSave and performDestroy

}

Do this with form requests too. TopicFormRequestInterface etc.

Then you can bind the correct implementation of the topic based on the first segment of the url. Use config to do the mapping.

# file : config/your_app_name.php

return [
  'topics' => [
    'supplier' => [
      'fqcn_model' => 'App\Entities\Supplier',
      'fqcn_form_request' => 'App\Http\Requests\SupplierFormRequest',
      'view_namespace' => 'resources/views/suppliers',
    ],
    // ...
  ],
];

Then you can bind the Model and FormRequests and register the view namespace based on the first segment of the url :

use Illuminate\Http\Request;
use Illuminate\Contracts\View\Factory as ViewFactory;

class AppServiceProvider extends ServiceProvider {

  public function register(Request $request, ViewFactory $view)
  {
    $config = config('your_app_name');

    $topic_token = $request->segment(1);

    $this->app->bind('App\Types\TopicInterface', $config['topic'][$topic_token]['fqcn_model']);
    $this->app->bind('App\Types\TopicFormRequestInterface', $config['topic'][$topic_token]['fqcn_form_request']);

    $view->prependNamespace('topics', [base_path($config['topic'][$topic_token]['view_namespace'])]);
  }

}

Then you can inject it in your TopicsController. And use the view namespace.

use App\Types\TopicInterface;
use App\Types\TopicFormRequestInterface;

class TopicsController extends controller{

  private $topic;

  public function __construct(TopicInterface $topic)
  {
    $this->topic = $topic;
  }

  public function index()
  {
    $topics = $this->topic->all();

    return view('topics::index', ['topics' => $topics]);
  }

  public function show($topic_token, $id)
  {
    $topic = $this->topic->findOrFail($id);

    return view('topics::show', ['topic' => $topic]);
  }

  public function save(TopicFormRequestInterface $request, $topic_token)
  {
    $topic = $this->topic->newInstance();

    $topic->performSave($request->all());

    redirect('topics.show', [$topic_token, $topic]);
  }

  // Etc etc....

}

Last thing you route all the topics to the topic controller :

$topic_tokens = array_keys(config('your_app_name.topics'));

$router->pattern('topic', implode('|', $topic_tokens));

$router->get('{topic}', ['as' => 'topics.index', 'uses' => 'TopicsController@index']);
$router->get('{topic}/{id}', ['as' => 'topics.show', 'uses' => 'TopicsController@show']);
// Etc etc....

If one of your Topic have a very specific implementation a particular action, you can create its own Controller, extends it from the TopicsController and override this specific action.

Please or to participate in this conversation.