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