lundi 24 décembre 2018

Improving a model attribute for better efficiency and usability

Framework: Laravel 5.1

Background: I have a Product and a ProductVariation model, which both share a ImplementsProductAttributes trait. If a Product is variable, than the product should contain at least a single purchasable ProductVariation to be purchasable, rest of the logic would be obvious within the code sample from the trait. Functionality works as is.

public function getPurchasableAttribute()
{
    if ($this instanceof \App\Product)
    {
        if ($this->type == 'variable')
        {
            foreach ($this->variations as $variation)
            {
                if ($variation->purchasable)
                    return true;
            }

            return false;
        }
        else return $this->isItemPurchasable();
    }
    else
    {
        return $this->isItemPurchasable();
    }
}

public function isItemPurchasable()
{
    if($this->regular_price > 0 AND $this->published)
    {
        if ($this->manage_stock) 
        {
            return $this->stock_quantity > 0;
        }
        else return $this->in_stock;
    }
    else return false;
}

Pros:

  • I make use of laravels attributes,
  • Readable/maintainable code,

Cons:

  • Redundant eager loading on the variations (which could be a problem if this was supposed to be used on an API)
  • Redundant SQL queries (I could implement a single query to get a count in the end, and decide based on that if the product itself is purchasable, but instead I am loading all variations -- at least until i get one with purchasable=true --)

Question: Should I keep this implementation or should I improve by getting rid of pulling relationships and making a simple SQL query to get the purchasable variations?

Please elaborate on your answer as much as you can.

Edit: Also please let me know if you think any other pros/cons exist.



via Chebli Mohamed

Aucun commentaire:

Enregistrer un commentaire