php is bad language to a lot of people, right? So, when I need to use php this is how I write it!

IndexController.php

<?php

namespace App\Http\Controllers\Web\Front;

use App\Http\Controllers\Controller;
use App\Repositories\AboutUsRepository;
use App\Repositories\BannerRepository;
use App\Repositories\ClientCategoryRepository;
use App\Repositories\ClientRepository;
use App\Repositories\CompanyInfoRepository;
use App\Repositories\ContactRepository;
use App\Repositories\EventRepository;
use App\Repositories\LocationRepository;
use App\Repositories\ManagementRepository;
use App\Repositories\MarketConcentrationRepository;
use App\Repositories\PortfolioCategoryRepository;
use App\Repositories\PortfolioRepository;
use App\Repositories\ProductRepository;
use App\Repositories\ResourceInfoRepository;
use App\Repositories\TestimonialRepository;

/**
 * Class IndexController
 * @package App\Http\Controllers\Web\Front
 */
class IndexController extends Controller
{
    /** @var CompanyInfoRepository */
    private $companyInfoRepository;

    /** @var ContactRepository */
    private $contactRepository;

    /** @var LocationRepository */
    private $locationRepository;

    /** @var BannerRepository */
    private $bannerRepository;

    /** @var ResourceInfoRepository */
    private $resourceInfoRepository;

    /** @var MarketConcentrationRepository */
    private $marketConcentrationRepository;

    /** @var AboutUsRepository */
    private $aboutUsRepository;

    /** @var ClientCategoryRepository */
    private $clientCategoryRepository;

    /** @var ClientRepository */
    private $clientRepository;

    /** @var ProductRepository */
    private $productRepository;

    /** @var PortfolioCategoryRepository */
    private $portfolioCategoryRepository;

    /** @var PortfolioRepository */
    private $portfolioRepository;

    /** @var TestimonialRepository */
    private $testimonialRepository;

    /** @var EventRepository */
    private $eventRepository;

    /** @var ManagementRepository */
    private $managementRepository;

    /**
     * IndexController constructor.
     * @param CompanyInfoRepository $companyInfoRepository
     * @param ContactRepository $contactRepository
     * @param LocationRepository $locationRepository
     * @param BannerRepository $bannerRepository
     * @param ResourceInfoRepository $resourceInfoRepository
     * @param MarketConcentrationRepository $marketConcentrationRepository
     * @param AboutUsRepository $aboutUsRepository
     * @param ClientRepository $clientRepository
     * @param ClientCategoryRepository $clientCategoryRepository
     * @param ProductRepository $productRepository
     * @param PortfolioCategoryRepository $portfolioCategoryRepository
     * @param PortfolioRepository $portfolioRepository
     * @param TestimonialRepository $testimonialRepository
     * @param EventRepository $eventRepository
     * @param ManagementRepository $managementRepository
     */
    public function __construct(CompanyInfoRepository $companyInfoRepository,
                                ContactRepository $contactRepository,
                                LocationRepository $locationRepository,
                                BannerRepository $bannerRepository,
                                ResourceInfoRepository $resourceInfoRepository,
                                MarketConcentrationRepository $marketConcentrationRepository,
                                AboutUsRepository $aboutUsRepository,
                                ClientRepository $clientRepository,
                                ClientCategoryRepository $clientCategoryRepository,
                                ProductRepository $productRepository,
                                PortfolioCategoryRepository $portfolioCategoryRepository,
                                PortfolioRepository $portfolioRepository,
                                TestimonialRepository $testimonialRepository,
                                EventRepository $eventRepository,
                                ManagementRepository $managementRepository)
    {
        $this->companyInfoRepository = $companyInfoRepository;
        $this->contactRepository = $contactRepository;
        $this->locationRepository = $locationRepository;
        $this->bannerRepository = $bannerRepository;
        $this->resourceInfoRepository = $resourceInfoRepository;
        $this->marketConcentrationRepository = $marketConcentrationRepository;
        $this->aboutUsRepository = $aboutUsRepository;
        $this->clientRepository = $clientRepository;
        $this->clientCategoryRepository = $clientCategoryRepository;
        $this->productRepository = $productRepository;
        $this->portfolioCategoryRepository = $portfolioCategoryRepository;
        $this->portfolioRepository = $portfolioRepository;
        $this->testimonialRepository = $testimonialRepository;
        $this->eventRepository = $eventRepository;
        $this->managementRepository = $managementRepository;
    }

    /**
     * @return \Illuminate\Contracts\Foundation\Application|\Illuminate\Contracts\View\Factory|\Illuminate\Contracts\View\View
     */
    public function index()
    {
        $companyInfo = $this->companyInfoRepository->first();
        $contact = $this->contactRepository->findPrimaryOrFirst();
        $location = $this->locationRepository->first();
        $banners = $this->bannerRepository->all()->sortBy('order');
        $resourceInfo = $this->resourceInfoRepository->first();
        $marketConcentrations = $this->marketConcentrationRepository->all()->sortBy('order');
        $aboutUs = $this->aboutUsRepository->first();
        $clientsCount = $this->clientRepository->count();
        $clientCategoriesWithClients = $this->clientCategoryRepository->categoriesWithOrderedClients();
        $products = $this->productRepository->all()->sortBy('order');
        $portfolioCategories = $this->portfolioCategoryRepository->all()->sortBy('order');
        $portfolios = $this->portfolioRepository->all()->sortBy('order');
        $testimonials = $this->testimonialRepository->all()->sortBy('order');
        $events = $this->eventRepository->findLatestPublishedEvents(3);
        $managements = $this->managementRepository->all()->sortBy('order');

        return view('web.front.index.index', compact(
            'companyInfo', 'contact', 'location', 'banners', 'resourceInfo', 'marketConcentrations', 'aboutUs',
            'clientsCount', 'clientCategoriesWithClients', 'products', 'portfolioCategories', 'portfolios',
            'testimonials', 'events', 'managements'
        ));
    }
}

It does look bad ☺️

It does look bad relaxed

@mahabubulhasan So, how it can be improved?

It does look bad relaxed

@mahabubulhasan So, how it can be improved?

Those comments are just redundant and verbose

The problem:
The pattern you are using is called "constructor injection". It is of course a good option, but when the dependency is growing it will look worse (given it looks unmanageable to a new set of eyes - pretty unreadable - and it implies the index controller is responsible for all the operations happening here breaking SRP).

Possible solutions:
(1)

• The index layout can be refactored in such a way, that the index class doesn't need that many repository dependencies at all
• Then each part of the layout can fetch their part of data either via client-side or by any other way (pre-compiled/component-based designs, etc)

(2)

• Distribute the responsibility of the whole data to parts of the views
• You can create another layer where you can build parts of the view elements (which will be built with their own sets of data) then call that layer (should we call it a service?!?) to render the final one.

(3)
..... and many more (need context to move forward than this level)

Personally, I would have gone for some component-based frameworks (like - reactjs or vuejs) in the front end if my backend code looks like this.

It does look bad relaxed

@mahabubulhasan So, how it can be improved?

Those comments are just redundant and verbose

Point noted!

The problem: The pattern you are using is called "constructor injection". It is of course a good option, but when the dependency is growing it will look worse (given it looks unmanageable to a new set of eyes - pretty unreadable - and it implies the index controller is responsible for all the operations happening here breaking SRP).

Possible solutions: (1)

* The `index` layout can be refactored in such a way, that the index class doesn't need that many repository dependencies at all

* Then each part of the layout can fetch their part of data either via client-side or by any other way (pre-compiled/component-based designs, etc)

(2)

* Distribute the responsibility of the whole data to parts of the views

* You can create another layer where you can build parts of the view elements (which will be built with their own sets of data) then call that layer (should we call it a service?!?) to render the final one.

(3) ..... and many more (need context to move forward than this level)

Personally, I would have gone for some component-based frameworks (like - reactjs or vuejs) in the front end if my backend code looks like this.

@himelnagrana Thanks and can't appreciate more!

For this specific code, I should have mention few more points.

HMVC can be a solution, perhaps?

Thing is,

1. It's a part of custom CMS for client. Not sure, this product will grow bigger or not. If needed, new architecture and refactor can be possible.
2. There will be "not so much data" is needed for this page.
3. Also, caching option is un-used till now.
4. Can't use vue, react for the front-facing web page(Client's requirement), as the page should be refreshed every-time, whenever it clicked!

Can you please share your opinion about HMVC? Doesn't it means more HTTP requests and it may be more costly? Reverse-proxy-caching is an option too but it will add another dependency. Concern about SRP too. But I think, I have to balance the cost(http request) and SRP like technical decision. Which process can be followed in such case?

Thanks in advance!

@himelnagrana To make it eye-friendly, I can think of only one solution till now and that is to create a Service class called, IndexPageService and pick necessary data from it. But again, It will surely break SRP.

and this is what it looks like without any business value:

namespace App\Http\Controllers\Web\Front;

use App\Http\Controllers\Controller;
use App\Services\Contracts\IndexPageContract;

class IndexController extends Controller
{
    private $indexPageService;

    public function __construct(IndexPageContract $indexPageService)
    {
        $this->indexPageService = $indexPageService;
    }

    public function index()
    {
        return view('web.front.index.index', $this->indexPageService->all());
    }
}

I'll refactor it when needed.

Thanks for mentioning me and receiving the review professionally and not personally. I really appreciate that.

First, as I mentioned earlier, without much context it is really hard to suggest/discuss a possible solution. But still, the last code snippet you shared is much much readable and debug-friendly. And if it was up to me, I wouldn't mind distributing the service class design also. That means I would have created wrapper service classes for each repository which would expose certain data (that is only relevant). Then accumulate all of them in IndexService class. That would not break SRP.

It might sound a bit elaborate and meaningless work -- but based on the size / usage or the data it would have been a good choice.
And of course, this is a personal opinion.

Secondly, I can't comment about HMVC without more context.

Thirdly, using client frameworks like vue/react would ease this problem actually. From the code, I am guessing and you shared too that there are lots of components/widgets that are interacted by users. What you are basically doing sending a request from browser to backend.

Yes true, there will be lots of HTTP calls. But that also can be minimized. And about reloading page - can very well be replaced with reloading segments/components/widgets of page. And it can be handled by client side libraries. You can check about web applications where the initial pages loads first and data in different sections loads asynchronously.

============

Having said all of this ---- PLEASE consider three things:

1. consider all of this as my personal opinion
2. without really knowing the application requirement and context, it is not very wise to comment about things like architecture and/or technology
3. Whole point of my commenting was you seemed like asking for opinion and I thought if I can help you with some idea -- so that you can read/explore some out of the box idea to solve your problems.

Good luck with that. 🙏🏼

@himelnagrana Thank you very much for your valuable time and input. Surely your input will help me to think about and yes, without knowing the specific requirement it's tough to suggest any specific solution. But as there are multiple ways of solving the same thing, I have to figure it out for this context.

Can't thank you much and that was a very good cross-check for me that can't be done without you guys. Appreciated!

Wishing you a very good day!

PHP is a good thing for people who are trying to learn programming languages. And this is an excellent example of bad code.

@shifatbuet Aah! yeah! It's me and not php.

You may suggest some code re-factoring in this situation. I want to hear from you that where it can be improved. You can forget about the context even.

@shifatbuet , So this is the 2nd part of "php is bad" and for "Beginners only". https://gist.github.com/codeperl/94ebdcb6b86d003e64da43fec31179a9

How did you do it in Java? Annotations? Do you want to check that version? I will suggest you to check Doctrine annotation class and you can do more just by extending that. It is there for at least last 7-8 years!

1. I see java code in procedural way! Forcing to write classes doesn't make someone "expert", "object oriented design" and that is how java is also forgiving and beginner friendly and that's okay.

   Comparing to java, php is more beginner friendly and forgiving but that does not mean it's a "Beginner only" language.

2. Even if someone is trying to learn programming with concept, best is to go for c and c++! Not java! If someone want to inspire himself for programming, python is very good choice, not php!

3. Once people used to develop "simple informative websites" with java or microsoft technology. php makes that easy! Most of the people hates because of that. They lost a lot of extra bucks and php was good enough for those "simple informative websites" they have no more chance to argue!

4. The best thing php core team did is, taking the criticism positively and constantly tries to improve that.

5. Scaling is a different issue which is not completely dependent on language.

6. You can't do something that doesn't mean you need to hate it. :)