DEV Community

loading...

Laravel: Refactoring Opportunity

jringeisen profile image Jonathon Ringeisen ・3 min read

I was adding a feature to the Saas business that I operate and I came across an opportunity to refactor some code and also learned something new which I would like to share.

The Feature

Within the software, a user can create a questionnaire form, generate a link for that form, and send the link to their client to be filled out. Once the questionnaire has been answered they can view the answers but they can't download them, so this feature added the ability to download the questions and answers to pdf.

Before Refactoring

The snippet of code that we're going to cover is the blade view that I am using to generate the pdf file.

$questionnaire->answered_form is a JSON column that we have to iterate through to try and match the questions with the answers.

@extends('guest.layouts.client_layout')

@section('title', 'Questionnaire')

@section('content')
    <div class="mt-12 px-4">
        <div class="w-full rounded-lg shadow-lg px-8 py-8 md:max-w-2xl md:mx-auto">
            @foreach($questionnaire->answered_form as $form)
                @if($form['type'] === 'text')
                <ul style="margin-top: 25px;list-style-type:none;">
                    <li>{{ $form['label'] }}</li>
                    <strong><li>{{ array_key_exists('userData', $form) ? $form['userData'][0] : '' }}</li></strong>
                </ul>
                @endif

                @if($form['type'] === 'number')
                    <ul style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        <strong><li>{{ array_key_exists('userData', $form) ? $form['userData'][0] : '' }}</li></strong>
                    </ul>
                @endif

                @if($form['type'] === 'textarea')
                    <ul style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        <strong><li>{{ array_key_exists('userData', $form) ? $form['userData'][0] : '' }}</li></strong>
                    </ul>
                @endif
                @if($form['type'] === 'checkbox-group') 
                    <ul class="mt-3" style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        @foreach($form['values'] as $value)
                            @if(array_key_exists('value', $value) && $value['value'] === $form['userData'][0])
                                <strong><li>{{ $value['label'] }}</li></strong>
                            @endif
                        @endforeach
                    </ul>
                @endif

                @if($form['type'] === 'select')
                    <ul class="mt-3" style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        @foreach($form['values'] as $value)
                            @if(array_key_exists('value', $value) && $value['value'] === $form['userData'][0])
                                <strong><li>{{ $value['label'] }}</li></strong>
                            @endif
                        @endforeach
                    </ul>
                @endif

                @if($form['type'] === 'radio-group')
                    <ul class="mt-3" style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        @foreach($form['values'] as $value)
                            @if(array_key_exists('value', $value) && $value['value'] === $form['userData'][0])
                                <strong><li>{{ $value['label'] }}</li></strong>
                            @endif
                        @endforeach
                    </ul>
                @endif
            @endforeach
        </div>
    </div>
@endsection

Enter fullscreen mode Exit fullscreen mode

If you look at the code you'll notice that the first three if statements are doing the same thing and the last three if statements are doing the same thing so this can easily be refactored to the following:

@extends('guest.layouts.client_layout')

@section('title', 'Questionnaire')

@section('content')
    <div class="mt-12 px-4">
        <div class="w-full rounded-lg shadow-lg px-8 py-8 md:max-w-2xl md:mx-auto">
            @foreach($questionnaire->answered_form as $form)
                @if($form['type'] === 'text' || $form['type'] === 'number' || $form['type'] === 'textarea')
                    <ul style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        <strong><li>{{ array_key_exists('userData', $form) ? $form['userData'][0] : '' }}</li></strong>
                    </ul>
                @endif

                @if($form['type'] === 'checkbox-group' || $form['type'] === 'select' || $form['type'] === 'radio-group')
                    <ul class="mt-3" style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        @foreach($form['values'] as $value)
                            @if(array_key_exists('value', $value) && $value['value'] === $form['userData'][0])
                                <strong><li>{{ $value['label'] }}</li></strong>
                            @endif
                        @endforeach
                    </ul>
                @endif
            @endforeach
        </div>
    </div>
@endsection
Enter fullscreen mode Exit fullscreen mode

We were able to reduce our if statements from 6 to only 2 if statements by using the || logical operator for or.

The above code can be refactored even more, after doing some research I found that you can reduce your if statements by using in_array(), like so:

@extends('guest.layouts.client_layout')

@section('title', 'Questionnaire')

@section('content')
    <div class="mt-12 px-4">
        <div class="w-full rounded-lg shadow-lg px-8 py-8 md:max-w-2xl md:mx-auto">
            @foreach($questionnaire->answered_form as $form)
                @if(in_array($form['type'], ['text', 'number', 'textarea']))
                    <ul style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        <strong><li>{{ array_key_exists('userData', $form) ? $form['userData'][0] : '' }}</li></strong>
                    </ul>
                @endif

                @if(in_array($form['type'], ['checkbox-group', 'select', 'type', 'radio-group']))
                    <ul class="mt-3" style="margin-top: 25px;list-style-type:none;">
                        <li>{{ $form['label'] }}</li>
                        @foreach($form['values'] as $value)
                            @if(array_key_exists('value', $value) && $value['value'] === $form['userData'][0])
                                <strong><li>{{ $value['label'] }}</li></strong>
                            @endif
                        @endforeach
                    </ul>
                @endif
            @endforeach
        </div>
    </div>
@endsection

Enter fullscreen mode Exit fullscreen mode

By using in_array() we're able to reduce the conditional inside the if statement to make it a little bit more readable.

As a developer you spend most of your time reading code, so making it as readable as possible and condensing it down when you can, goes a long way in readability and making life easier on the person reading the code.

Comment below if you see something else that can be refactored.

Discussion (0)

Forem Open with the Forem app