patterncsharpMinor
Shopping cart code kata
Viewed 0 times
codekatacartshopping
Problem
I have recently started code kata and learning for myself, as I would like to
learn test-first development. I have started doing a shopping cart as my code kata. I have included a test as well. I would like to know what are the ways I could improve my below code. I know I need to use dependency injection next step. If you find anything I can improve in my code, please let me know.
```
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
namespace Domain.Tests.Shopping_Cart_Tests
{
[TestFixture]
public class ShoppingCartTest
{
private IShoppingCart _shoppingCart;
[SetUp]
public void SetUp()
{
_shoppingCart = new ShoppingCart();
}
[Test]
public void ShouldBeAbleToAddToCart()
{
var item1 = new CartItem{ProductId="P1001",Name = "Logitech Mouse", Qty = 1, Price = 5.00};
_shoppingCart.AddToCart(item1);
Assert.AreEqual(_shoppingCart.GetCartItems().Count, 1);
}
[Test]
public void ShouldBeAbleToAddToCartMultipleItems()
{
var item1 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 1, Price = 5.00 };
var item2 = new CartItem { ProductId = "P1002", Name = "Logitech Keyboard", Qty = 1, Price = 9.00 };
_shoppingCart.AddToCart(item1);
_shoppingCart.AddToCart(item2);
Assert.AreEqual(_shoppingCart.GetCartItems().Count, 2);
}
[Test]
public void ShouldBeAbleToIncrementWhenAddTheSameItem()
{
var item1 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 1, Price = 5.00 };
var item2 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 2, Price = 5.00 };
var item3 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 3, Price = 5.00 };
_shoppingCart.CheckItemExistThenAdd
learn test-first development. I have started doing a shopping cart as my code kata. I have included a test as well. I would like to know what are the ways I could improve my below code. I know I need to use dependency injection next step. If you find anything I can improve in my code, please let me know.
```
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
namespace Domain.Tests.Shopping_Cart_Tests
{
[TestFixture]
public class ShoppingCartTest
{
private IShoppingCart _shoppingCart;
[SetUp]
public void SetUp()
{
_shoppingCart = new ShoppingCart();
}
[Test]
public void ShouldBeAbleToAddToCart()
{
var item1 = new CartItem{ProductId="P1001",Name = "Logitech Mouse", Qty = 1, Price = 5.00};
_shoppingCart.AddToCart(item1);
Assert.AreEqual(_shoppingCart.GetCartItems().Count, 1);
}
[Test]
public void ShouldBeAbleToAddToCartMultipleItems()
{
var item1 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 1, Price = 5.00 };
var item2 = new CartItem { ProductId = "P1002", Name = "Logitech Keyboard", Qty = 1, Price = 9.00 };
_shoppingCart.AddToCart(item1);
_shoppingCart.AddToCart(item2);
Assert.AreEqual(_shoppingCart.GetCartItems().Count, 2);
}
[Test]
public void ShouldBeAbleToIncrementWhenAddTheSameItem()
{
var item1 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 1, Price = 5.00 };
var item2 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 2, Price = 5.00 };
var item3 = new CartItem { ProductId = "P1001", Name = "Logitech Mouse", Qty = 3, Price = 5.00 };
_shoppingCart.CheckItemExistThenAdd
Solution
public double Price { get; set; }clearly should be of typeDecimal. That is whatDecimalfor.
private static List CartItems = null;can be safely refactored toDictionarywithProductIdused as keys. ThenCheckItemExistThenAddToCartcan be refactored usingDictionary.ContainsKeymethod, andGetCartItemscan return dictionary itself,CartItems.ValuesorCartItems.Values.ToList(). I think the latter option is the best, since it will copy the collection, which will restrict access toCartItemsproperty. As of now, someone might modify it in outer code, which will result it modifying ShoppingCart state. Returning a copy will prevent that.
return (CartItems as IEnumerable).GetEnumerator();cast can be removed.
- I'm not sure i like the idea of having two methods to add items to the cart, which have different logic. It's pretty error-prone. Is it really necessary? I think you should probably stick to one.
- I think your shopping cart interface misses
Removemethod.
Your unit tests look fine to me (except you should move them to different file), tho i'm not too experienced with them.
Context
StackExchange Code Review Q#30600, answer score: 2
Revisions (0)
No revisions yet.