vanilophp / cart

Cart Module for Vanilo (or any Laravel app)
https://vanilo.io
MIT License
50 stars 17 forks source link

Uses cart property directly and stops wondering if it exists or not #2

Closed druc closed 6 years ago

druc commented 6 years ago

Hi there, I'm working on a small e-commerce site for a client and I found myself looking for inspiration and stumbled upon your project.

I'm not using the vanilophp framework yet (still getting familiar with concord) but I am picking some pieces from here and there and I thought I should share any possible improvements/additions/fixes I might find.


The PR:

Removes the whole wondering if the cart exists and calls it directly, so no more "if exists" statements.

Doesn't go around and starts creating cart database records but when removeItem(), removeProduct(), clear(), itemCount(), total(), delete() are called, the queries are indeed executed (using cart_id = null) even if the cart record isn't yet created.

But, I don't see how these methods will be called if no cart is yet created.

If you do think there's a use-case for that but still like the idea, we could add a nullable cart object that will prevent touching the database.

If you don't like the idea at all, feel free to turn down the pr - I thought it's worth sharing.

fulopattila122 commented 6 years ago

Hello Druc,

thanks for the PR.

The reason of CartManager being present, and being above the Cart ActiveRecord is to avoid creating millions of empty carts. (for every new session, even for bots)

I saw this happening in many ecommerce sites. Not only creates literally millions of carts in the database but also adds a surplus SQL query to every single request.

As far as I see what your PR does is that it makes the CartManager pass down every request to the ActiveRecord, as if you were using the Cart Active Record directly.

This is something you can absolutely do, just use the Vanilo\Cart\Models\Cart instead of the Cart facade.

druc commented 6 years ago

@fulopattila122 That does make a lot of sense! Thanks for explaining it to me 👍