Laravel: Refactoring Opportunity
Jonathon Ringeisen
Posted on April 26, 2021
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
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
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
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.
Posted on April 26, 2021
Join Our Newsletter. No Spam, Only the good stuff.
Sign up to receive the latest update from our blog.