1
\$\begingroup\$

I am working on a blogging application in Laravel 8.

In the FrontendController controller I have:

namespace App\Http\Controllers;
use App\Models\Settings;
use App\Models\ArticleCategory;
class FrontendController extends Controller
{
    protected $site_settings;
    protected $theme_directory;
    protected $site_name;
    protected $tagline;
    protected $owner_name;
    protected $article_categories;

    public function __construct()
    {
        $this->site_settings = Settings::first();
        $this->theme_directory = $this->site_settings['theme_directory'] ?? null;
        $this->site_name = $this->site_settings['site_name'] ?? null;
        $this->tagline = $this->site_settings['tagline'] ?? null;
        $this->owner_name = $this->site_settings['owner_name'] ?? null;


        // Article categories
        $this->article_categories = ArticleCategory::all();
    }
}

The ArticlesController controller extends the one above:

namespace App\Http\Controllers;
use Illuminate\Http\Request;
use App\Models\ArticleCategory;
use App\Models\Article;

class ArticlesController extends FrontendController {

    // Articles per page
    protected $per_page = 12;

    public function index(Request $request) {

        // Search query
        $qry = $request->input('search');

        $articles = Article::where('title', 'like', '%' . $qry . '%')
                                                ->orWhere('short_description', 'like', '%' . $qry . '%')
                                                ->orWhere('content', 'like', '%' . $qry . '%')
                                                ->orderBy('id', 'desc')
                                                ->paginate($this->per_page);

        // Search results count
        if ($request->input('search')){
            $article_count = Article::where('title', 'like', '%' . $qry . '%')
                                                ->orWhere('short_description', 'like', '%' . $qry . '%')
                                                ->orWhere('content', 'like', '%' . $qry . '%')
                                                ->count();
        }       

        return view('themes/' . $this->theme_directory . '/templates/index', 
            [
                'theme_directory' => $this->theme_directory,
                'search_query' => $qry,
                'site_name' => $this->site_name,
                'tagline' => $this->tagline,
                'owner_name' => $this->owner_name,
                'categories' => $this->article_categories,
                'articles' => $articles,
                'article_count' => $article_count ?? null
            ]
        );
    }

    public function category($category_id) {
        $category = ArticleCategory::where('id', $category_id)->first();
        $articles = Article::where('category_id', $category_id)->paginate($this->per_page);

        return view('themes/' . $this->theme_directory . '/templates/index', 
            [
                'theme_directory' => $this->theme_directory,
                'site_name' => $this->site_name,
                'tagline' => $this->tagline,
                'owner_name' => $this->owner_name,
                'categories' => $this->article_categories,
                'category' => $category,
                'articles' => $articles
            ]
        );
    }

    public function show($slug) {
        // Single article
        $article = Article::where('slug', $slug)->first();
        return view('themes/' . $this->theme_directory . '/templates/single', 
            [
                'theme_directory' => $this->theme_directory,
                'site_name' => $this->site_name,
                'tagline' => $this->tagline,
                'owner_name' => $this->owner_name,
                'categories' => $this->article_categories,
                'article' => $article
            ]
        );
    }
    
}

The posts list view (index.blade.php):

@extends('themes/' .$theme_directory . '/layout')

@section('content')

<!-- Page Header -->
<header class="masthead" style="background-image: url({{ asset('themes/' . $theme_directory . '/img/home-bg.jpg') }}">
    <div class="overlay"></div>
    <div class="container">
        <div class="row">
            <div class="col-lg-8 col-md-10 mx-auto">
                <div class="site-heading">
                    <h1>{{ $site_name }}</h1>
                    <span class="subheading">
                        @if(isset($category))
                            {{ $category->name }}
                        @else
                            {{ $tagline }}
                        @endif
                    </span>
                </div>
            </div>
        </div>
    </div>
</header>

    <div class="container">
    <div class="row">
      <div class="col-lg-8 col-md-10 mx-auto">

                @if (isset($search_query))
                    <p class="mt-0 text-muted">We found {{ $article_count }} posts containing <span class="quote-inline">{{ $search_query }}</span>:</p>
                @endif

                @if (count($articles))
                    @foreach ($articles as $article)
                        <div class="post-preview">
                            <a href="{{ url('/show/' . $article->slug) }}">
                                <h2 class="post-title">
                                    {{ $article->title }}
                                </h2>
                                <h3 class="post-subtitle">
                                    {{ $article->short_description }}
                                </h3>
                            </a>
                            <p class="post-meta">Posted by
                                <a href="#">{{ $article->user->first_name }} {{ $article->user->last_name }}</a>
                                on {{ date('j F, Y', strtotime($article->created_at)) }}
                            </p>
                        </div>
                        <hr>
                    @endforeach
                @endif

                <!-- Pager -->
                @if($articles->hasPages())
                    <div class="clearfix">
                        <ul class="pagination">
                            <li class="next">
                                <a class="btn btn-primary {{ $articles->withQueryString()->onFirstPage() ? 'disabled' : '' }}" href="{{ $articles->previousPageUrl() }}">&larr; Newer Posts</a>
                            </li>
                            <li class="prev">
                                <a class="btn btn-primary {{ $articles->withQueryString()->onLastPage() ? 'disabled' : '' }}" href="{{ $articles->nextPageUrl() }}">Older Posts &rarr;</a>
                            </li>
                        </ul>
                    </div>
                @endif
      </div>
    </div>
  </div>
  <hr>
@endsection

Questions

  1. What would be an optimal way to reduce code repetition in the two controllers?
  2. Are there any code optimisation opportunities?
\$\endgroup\$
0

2 Answers 2

2
+50
\$\begingroup\$

What would be an optimal way to reduce code repetition in the two controllers?

If those four values from the first settings record and $article_categories are applicable to all views then they could be shared with all views using the View facade's share() method in the boot() method of the App\Providers\AppServiceProvider. Then there may not be a need to have those lines in the constructor of the FrontEndController, and the member variables can be eliminated also.

Otherwise if those values from the settings record only apply to certain views, one could define a helper method(s) to obtain the data to be sent to the view. In the two methods index() and show() the first four entries are repeated so those could be returned by a new helper method, and the other data entry - e.g. articles or article can be added via array_merge() or the array union operator - i.e. +.

In the FrontEndController constructor a loop could be used to iterate over the properties that need to be set from the settings record.


In the ArticlesController::show() method these three lines could be pulled out:

Article::where('title', 'like', '%' . $qry . '%')
    ->orWhere('short_description', 'like', '%' . $qry . '%')
    ->orWhere('content', 'like', '%' . $qry . '%')

and assigned to a local variable like $articlesQuery, which could be then used to generate $articles and $article_count.


Are there any code optimisation opportunities?

If site_settings is only used inside the constructor then perhaps it does not need to be a member variable - it could simply be a local variable.


In the ArticlesController::category() method the line with the query to get the category:

$category = ArticleCategory::where('id', $category_id)->first();

Could be simplified using the firstWhere() method:

$category = ArticleCategory::firstWhere('id', $category_id);

The same applies to first line of the show() method - i.e. to find article by slug.


Does ArticlesController::$per_page ever get assigned a different value than 12? If not then it could be declared as a constant.


Variable parsing - also known as String interpolation - could be used to simplify instances of '%' . $qry . '%' to ”%$qry%”.

\$\endgroup\$
1
  • \$\begingroup\$ All the views in the theme(s) share FrontendController at the top of the front-end controller. Great advice. Thanks! \$\endgroup\$ Commented Aug 3, 2022 at 20:26
2
\$\begingroup\$

Ideas on the design:

Since Settings is a Model, and finally returned in the response with some configurations, and structuring. I would definitely encourage you to have it as an API resource. Where you can set the data and have it in the desired structure/format, which you are using the FrontEndController for, but it does seem to be a resource functionality IMO. read here the documentation: https://laravel.com/docs/9.x/eloquent-resources

Another point, check out the Repository Pattern. Where you'd have a separate layer for interacting with the database (retrieve, create.. etc). So you'd have a repository for the Settings or Articles for example, fetch them, and hand them over to their Resources to assemble them as you wish for the response.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.