How to reduce condition from Model | Write Clean Code - Part 2
In this series to tutorials, my plan is to take some real-life code and try to refactor the code as much as I can. Today, I will show you how to reduce unnecessary condition from the Laravel Model.
Previous Posts:
Case Study
I have a Laravel model where I feel there is some unnecessary code. I think this code can be improved more. Here is the code-
class CreditCardApplication extends Model
{
protected $table = 'credit_card_application';
protected $primaryKey = 'credit_card_application_id';
protected $fillable = [
'credit_card_application_id',
'shop_id',
'status',
'created_by',
'reason'
];
const STATUS_PENDING = 0;
const STATUS_APPROVED = 1;
const STATUS_REJECTED = 2;
const FREEZE_DAYS_AFTER_REJECTED = 7;
const STATUS_DISPLAY = [
self::STATUS_PENDING => "Pending",
self::STATUS_REJECTED => "Rejected",
self::STATUS_APPROVED => "Approved",
];
public function allowToResubmit($shopId)
{
if ( (int) $this->status == self::STATUS_REJECTED && $this->updated_at < Carbon::now()->subDays(self::FREEZE_DAYS_AFTER_REJECTED)){
return true;
}
$creditCardApplication = $this->getCreditCardApplication($shopId);
if (! $creditCardApplication){
return;
}
if ( (int) $creditCardApplication->status == self::STATUS_REJECTED && $creditCardApplication->updated_at < Carbon::now()->subDays(self::FREEZE_DAYS_AFTER_REJECTED)){
return true;
}
}
public function notAllowedToResubmit($shopId)
{
if ($this->allowToResubmit($shopId) !== true){
return true;
}
}
public function getCreditCardApplication($shopId)
{
return $this->where('shop_id', $shopId)->first();
}
}
Beside storing some const
, responsibilities of this model are-
- Whether a user is able to resubmit the application for a credit card or not?
- Return credit card application data.
Refactoring
First of all, I can see some unnecessary logic in allowToResubmit()
method. I think we can improve this method. Here is my improvement.
public function canResubmit()
{
if ( (int) $this->status == self::STATUS_REJECTED && $this->updated_at < Carbon::now()->subDays(self::FREEZE_DAYS_AFTER_REJECTED)){
return true;
}
return false;
}
I believe both of the methods are doing the same thing. Isn’t that?
So, I mainly come out with a new method name (perhaps) and refactored the code. The refactored model looks like that-
<?php
namespace App;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Model;
class CreditCardApplication extends Model
{
protected $table = 'credit_card_application';
protected $primaryKey = 'credit_card_application_id';
protected $fillable = [
'credit_card_application_id',
'shop_id',
'status',
'created_by',
'reason'
];
const STATUS_PENDING = 0;
const STATUS_APPROVED = 1;
const STATUS_REJECTED = 2;
const FREEZE_DAYS_AFTER_REJECTED = 7;
const STATUS_DISPLAY = [
self::STATUS_PENDING => "Pending",
self::STATUS_REJECTED => "Rejected",
self::STATUS_APPROVED => "Approved",
];
public function canResubmit()
{
if ( (int) $this->status == self::STATUS_REJECTED && $this->updated_at < Carbon::now()->subDays(self::FREEZE_DAYS_AFTER_REJECTED)){
return true;
}
return false;
}
}
Here, keep it in your mind that, I just used canResubmit()
method that can serve both purposes, can or cannot since it returns boolean
.
If you have a different idea, Feel free to share your though.
Thank you. :)