DO Repeat Yourself (Laravel Edition)

This story was originally published on beqode.com with proper code syntax highlighting and formatting. Read it there, give it a clap here.

OK, the title is clickbait, go against the “Don’t Repeat Yourself” (DRY) principle and you’ll trigger the Internet.

Obviously we don’t want to repeat ourselves a.k.a copy/paste, a.k.a spaghetti code ™, a.k.a “Don’t touch this piece”, a.k.a high maintenance cost code…

But there are (few) areas where it is even recommended to repeat yourself and I wanted to highlight them only because I made the mistakes myself when I was a junior developer.

Controllers

Here I refer to a controller as the method inside a class controller, the direct “controller” of a view, resource, etc…

Controllers can repeat themselves.

Well, that is if and only if you’ve been separating your domains properly:

Repositories to query your DB/Model
Services to extract your business logic

Once everything is separated, it is quite normal for 2 controllers to look alike:

class UserController
{
    protected $users;

    public function __construct(Users $users)
    {
        $this->users = $users;
    }

    public function show(Request $request, string $username)
    {
        $user = $this->users->findByUsername($username);

        if (!$user) {
            return abort(404);
        }

        return view('user.show', compact('user'));
    }

    public function edit(Request $request, string $username)
    {
        $user = $this->users->findByUsername($username);

        if (!$user) {
            return abort(404);
        }
        return view('user.edit', compact('user'));
    }
}

It’s obviously OK to repeat:

$user = $this->users->findByUsername($username);

if (!$user) {
    return abort(404);
}

Whereas if you did not have the Users repository, that would not be acceptable:

class UserController 
{
    public function show(Request $request, string $username)

    {
        $user = User::with('posts')
            ->whereNotNull('email_verified_at')
            ->whereNull('banned_at')
            ->where('username', $username)
            ->first();

        // ...
    }

    public function edit(Request $request, string $username)
    {
        $user = User::with('posts')
            ->whereNotNull('email_verified_at')
            ->whereNull('banned_at')
            ->where('username', $username)
            ->first();

        // ...
    }
}

To make my point even clearer: you should NOT attempt to group show and edit together in order to stay DRY:

class UserController
{
    // ...

    public function show(Request $request, string $username)
    {
        return $this->showOrEdit($request, $username, 'show');
    }

    public function edit(Request $request, string $username)
    {
        return $this->showOrEdit($request, $username, 'edit');
    }

    protected function showOrEdit(Request $request, string $username, string $action)
    {
        $user = $this->users->findByUsername($username);

        if (!$user) {
            return abort(404);
        }

        return view('user.' . $action, compact('user'));
    }
}

While this could seem like a good idea now (it is not), it is very likely the 2 controllers will evolve their own separate ways and you’ll end up with a showOrEdit full of if and very prone to errors ! It’s basically added complexity for very little gains and at the cost of readability.

Even worse, trying to group edit and show together inside the controller instead of properly extracting the business logic into the Users repository !

class UserController
{
    // ...

    protected function showOrEdit(Request $request, string $username, string $action)
    {
        $user = User::with('posts')
            ->whereNotNull('email_verified_at')
            ->whereNull('banned_at')
            ->where('username', $username)
            ->first();
        // ...
    }
}

So do repeat your controllers, but only when they are composed with your different repositories & services.

You could even OOP’ing your class controllers (inheritance) in order to group functionalities and stay DRY, this is almost certainly a bad idea ! class controllers are merely connectors to your views and should not hold any business logic (which is what the inheritance pattern is likely trying to achieve).

All of the above comes down to the “Fat model, skinny controllers” mantra. You can have as many skinny controllers as you want, and because they are skinny, because all your business logic (repositories, services) has been extracted, they will be very similar and that’s completely fine ! Do repeat those skinny controllers!

Views

Another area where you shouldn’t sweat it too long for keeping your code DRY is Views. One with good intentions could spend way too much time trying to re-use views, splitting them into multiple partials, make use of layout inheritance, etc…

Views are very very likely to change over time (copy, CSS…) and trying to optimise reusability is in my experience/opinion, not worth the effort.

Views are very unique by definition, they fulfil a very specific (display) purpose. While it seems like 2 similar views could be grouped together in a partial, it’s rarely true, misleading and will almost certainly end up in unmaintainable code.

See Views as disposable pieces of code: Copy them, delete them, replace them without too much thinking.

A concrete example is trying to make one unique component for two different list items.

Say you have a _booking-item.twig partial, it displays a Booking item in a list of upcoming bookings in your Admin area:

<article class="booking">
    <h2 class="booking__title">New Booking #{{ booking.id }}</h2>
    <div class="booking__buttons">
        <button>Accept</button>
        <button>Reject</button>
    </div>
</article>

Now you have a new requirement to display a list of archived bookings in a tab next to your upcoming bookings list.

It is tempting to reuse _booking-item.twig and to write some conditional logic in it:

<article class="booking{% if booking.isArchived() %} booking--archived{% endif %}">
    {% if booking.isUpcoming() %}
        <h2 class="booking__title">New Booking #{{ booking.id }}</h2>
    {% else %}
        <h2 class="booking__title">Booking #{{ booking.id }}</h2>
    {% endif %}
    {% if booking.isArchived() %}
        <p>Processed on {{ booking.processed_at|date('M d, Y') }}</p>
    {% endif %}
    {% if booking->isUpcoming() %}
        <div class="booking__buttons">
            <button>Accept</button>
            <button>Reject</button>
        </div>
    {% endif %}
</article>

While clean-ish, this is not (at all) scalable.

Also remember that the above is not a real world example, and I would even argue that this simple view is already bloated with needless logic that prevents me from understanding the final result of the view.

When I open a view I want to instantly have a feeling of what it will look like. Views are unimportant pieces of code, they are swappable, deletable, replaceable…

If we rewrite our _booking-item.twig partial into two different partials:

{# resources/views/_booking-item.twig (Unchanged) #}
<article class="booking">
    <h2 class="booking__title">New Booking #{{ booking.id }}</h2>
    <div class="booking__buttons">
        <button>Accept</button>
        <button>Reject</button>
    </div>
</article>
{# resources/views/_booking-archive-item.twig #}
<article class="booking booking--archived">
    <h2 class="booking__title">Booking #{{ booking.id }}</h2>
    <p>Processed on {{ booking.processed_at|date('M d, Y') }}</p>
</article>

I have a much better understanding of what the final result will look like.

Yes, if you change the booking__title class name in your CSS to booking__heading, you do have to edit 2+ files to change the class name in your views. That’s the trade-off for readability.

There is no hard line of when to group or split your views, but in my experience don’t be afraid to repeat yourself in your views, this is very much unimportant.

The point I am trying to make is: Views may not be DRY. I would even say they should not be DRY. This will save you a lot of time, again, Views are disposable, write them, leave them, come back to them, edit them, delete them, they should not be in the way, they should not delay the greater Architecture you have to design in your business logic, they must take very little of your precious bandwidth.