How to reduce condition from Model | Write Clean Code - Part 2

31 Jul 2019 . Laravel . 537 views

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. :)