I second Tim's comment regarding holding onto calculated values.
That's at best a performance optimization, and likely an unnecessary
one.
Also, by using the product itself as a key simplifies the case where
you try to "add" the same product as multiple line-items (though this
may be what you want if there are other considerations at play).
But as to the code you have, I'd suggest using some higher-level
functions. For example, change this:
(defn add-line-item [cart line-item]
(assoc cart
:line-items (conj (cart :line-items) line-item)
:total (+ (cart :total) (line-item :subtotal))))
to this:
(defn add-line-item [c li]
(-> c (update-in [:line-items] conj li)
(update-in [:total] + (:subtotal li))))
Likewise, you could change this:
(defn update-line-item [line-items product qty]
(cond
(empty? line-items) ()
(= ((first line-items) :product) product)
(cons
(assoc (first line-items)
:qty qty
:subtotal (* qty (((first line-
items) :product) :price)))
(rest line-items))
:else
(cons (first line-items)
(update-line-item (rest line-items) product qty))))
to this:
(defn update-line-item [cart product qty]
(-> cart (assoc :line-items (filter #(= product (:product %)) cart))
(add-line-item (create-line-item product qty))))
Etc...
On Oct 27, 4:41 am, Timothy Pratley <timothyprat...@gmail.com> wrote:
> Hi Robert
>
> On Oct 27, 9:48 pm, Robert Campbell <rrc...@gmail.com> wrote:
>
> > Hey guys, I'm looking for _any_ feedback/thoughts on this Clojure code
> > I wrote. I just feel like the entire thing is way too complex, but I'm
> > not sure about how to simplify it. I wanted to try something "real
> > world" so I made a simple shopping cart ref to put in a session:
>
> Great, an open invitation!
>
> structs are really no different from maps except as a performance
> optimisation (and not a huge one). So dropping the structs would
> remove some boilerplate if simplicity is your goal. Also why not make
> the cart a map of products to qty and forget about subtotal...
> subtotal and total are easily calculated by separate functions for
> view or checkout... something like (untested at all):
>
> (defn add-to-cart [product qty]
> (if (pos? qty)
> (dosync (alter cart update-in [product] #(+ qty (if % % 0)))))
>
> (defn update-cart [product qty]
> (dosync (alter cart assoc product qty)))
>
> (defn remove [product]
> (dosync (alter cart dissoc product)))
>
> (defn subtotal [product]
> (* (@cart product) (price product)))
>
> (defn total []
> (reduce + (map subtotal @cart)))
>
> But in a real-world example I'm not sure a ref would be the best way
> to deal with the state... wouldn't you have the cart sent up in the
> request and a new cart returned? ie: you wouldn't need to maintain a
> ref on the server, just provide the hooks for building a cart and
> checking out. So the functions might be better if they take a cart as
> input and return a cart. Doing that pretty much makes them empty
> functions:
> (defn remove [cart product]
> (dissoc cart product))
> So do you even need a remove function? Maybe not.
>
> Just some thoughts - I'm no web-shop programmer so disclaimer
> attached.
>
> Regards,
> Tim.
>
> Regards,
> Tim.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
-~----------~----~----~----~------~----~------~--~---
