Ayende @ Rahien

Ayende Rahien commented on Porting the MVC Music Store to Raven: ShoppingCartController

Sun, 30 May 2010 17:26:29 GMT

cart.GetCount() was a database query, _very_ expensive. I don't care about the actual impl detail, I care about whatever this is an in memory operation or a remote operation

Michael Valenty commented on Porting the MVC Music Store to Raven: ShoppingCartController

Sun, 30 May 2010 15:33:01 GMT

Why would you prefer cart.Lines.Count over cart.GetCount()? cart.Lines.Count is a Law of Demeter violation. You don't encapsulate the logic that @David pointed out should actually be cart.Sum(x => x.Quantity). Maybe it should be a property to suggest it's not an expensive operation, but it seems like it's something the cart should own.

Michel commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 21:36:05 GMT

For the DeleteId value, HTML id can't start with a number ( [http://validator.w3.org/docs/errors.html#ve-122](http://validator.w3.org/docs/errors.html#ve-122)). So instead of returning the number part of the id, you should return the full id minus the "/" : DeleteId = id.Replace('/', string.Empty)

Harry Steinhilber commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 19:06:48 GMT

@Michael, The point was that ShoppingCart.AddToCart() does not contain any data access. The data access is in the ShoppingCartController.AddToCart(). In the original version, the data access was in the business object instead of the controller.

Michael Teper commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:56:44 GMT

Ayende, Kiliman, you are both right, I missed the ShoppingCart reference. Thanks!

Kiliman commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:53:53 GMT

Oops... sorry for the double post. I didn't expect Ayende to moderate my comment so quickly. Thanks!

Ayende Rahien commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:53:21 GMT

Michael, Sure it is, but it is not in the shopping cart code

Ayende Rahien commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:52:01 GMT

Json, Post/redirect/get is much better option, sure.

Kiliman commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:51:16 GMT

@Michael, I believe he's means the ShoppingCart model does not include any data access code. See his previous post: "Porting the MVC Music Store to Raven: ShoppingCart" (sorry, edited due to link being consider spam.. ugh!) The calling code, in this case the controller, is responsible for loading and persisting the model. I suppose you could add another level of abstraction by introducing a ShoppingCartService that managed the data access instead of the controller, but for a simple application, this is probably overkill.

Ayende Rahien commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:51:12 GMT

David, You are correct

Michael Teper commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:17:37 GMT

Ugn, subtext ate the generic part. Here's what I meant: Wouldn't session.Load(id) call be data access?

Michael Teper commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 18:16:35 GMT

Wouldn't session.Load

josh commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 16:40:43 GMT

@David Depends on what you are intending to show. The line count or the total item count.

Jason Meckley commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 16:21:19 GMT

an approach I use with controller's is post-redirect-get for commands. by managing transactions (SaveChanges) from a Filter there is no need to call session.SaveChanges() in the action. you also avoid the "resend post data" if the user refreshes the page. in this series you are only porting the data access, so keeping the controllers as similar as possible makes sense. What are your thoughts?

David Thibault commented on Porting the MVC Music Store to Raven: ShoppingCartController

Thu, 27 May 2010 15:57:12 GMT

I believe that shoppingCart.Lines.Count should be shoppingCart.Lines.Sum(x => x.Quantity) However I did not look at the original code so it may be correct.