使方法更少的代码行

我有一个像下面的方法，有没有什么办法重构，更清洁的方式，以便我可以使它在更少的代码行中，例如去除if/for循环，就像那样使方法更少的代码行

public void CheckProductExistThenAddToCart(CartItem item) 
{ 
    if (CartItems.Count == 0) AddToCart(item); 

    bool itemFound = false; 
    foreach (var cartItem in CartItems) 
    { 
     if (cartItem.ProductId.Equals(item.ProductId)) 
     { 
      itemFound = true; 
      cartItem.Qty += item.Qty; 
      break; 
     } 
    } 

    if (!itemFound) 
    { 
     AddToCart(item); 
    } 
}

来源

2013-08-31 mugstack

这看起来会更好放在http://codereview.stackexchange.com – Gjeltema

使用LINQ，正如Reed Copsey的答案中所给出的，会显着减少你的代码行。不过要注意的是，更少的代码行实际上可以非常严重地降低代码的可读性/可维护性。换句话说：如果一行代码需要文档，以便理解它的功能，而两行代码将不言自明，请转至两行代码。 “每条线路的大部分功能都没有支付。”您支付正确，可读和可维护的代码。 –

@JanDoerrenhaus真的 - 虽然我认为我的版本比原来的版本更具可读性和可维护性... –

你可以使用LINQ：

public void CheckProductExistThenAddToCart(CartItem item) 
{ 
    var existingItem = CartItems.FirstOrDefault(ci => ci.ProductID == item.ProductId); 
    if (existingItem == null) 
      CartItems.Add(item); 
    else 
      existingItem.Qty += item.Qty; 
}

来源

2013-08-31 23:07:08

注意重复的'item'变量名称。 – Douglas

非常感谢 – mugstack

@Douglas谢谢 - 固定 –

如果有独特的项目（在ProductId的上下文中）应保证可以使用SingleOrDefault。如果可能有多个并且您想忽略此事实，请更改为FirstOrDefault。我发现Single更好，因为它在这里明确表达意图。

public void CheckProductExistThenAddToCart(CartItem item) 
{ 
    var existingItem = CartItems 
     .SingleOrDefault(i => i.ProductId.Equals(item.ProductId)); 

    if (existingItem == null) 
    { 
    AddToCart(item); 
    } 
    else 
    { 
    existingItem.Qty += item.Qty; 
    } 
}

来源

2013-08-31 23:07:59 BartoszKP

感谢您的即时回复 – mugstack

为了缩短这个功能，你可以考虑使用

Dictionary<ProductId, CartItem> dict;

然后，而不是通过车循环的只是使用

if (dict.ContainsKey(productId)) 
{ 
    // add qty 
} else { 
    // add item to cart 
}

来源

2013-08-31 23:10:05

字典使这种方法变得简单得多，但可能会使其他处理更加复杂。例如，字典不会保留订单，大多数人会抱怨，如果添加一个项目到你的购物车洗牌项目的顺序等。 –

当然，但通常我不使用普通的收藏品用于此目的。我将从一个集合开始，然后把它放在一个能帮助我组织问题的课程中。例如您可以添加一个数组，以便在您需要将其放到屏幕上时订购购物车。 –

首先，有一个bug，因为你是在添加缺少的项目后不返回。因此，您将Qty添加到您之前刚刚添加的相同项目，因此它的价值翻了一番。

所以不是：

public void CheckProductExistThenAddToCart(CartItem item) 
{ 
    if (CartItems.Count == 0) AddToCart(item); 
    // missing return 

    bool itemFound = false; 
    foreach (var cartItem in CartItems) 
    { 
     if (cartItem.ProductId.Equals(item.ProductId)) 
     { 
      itemFound = true; // ypu will find the same item you have justb added 
      // ... causes this bug and is less efficient 
      cartItem.Qty += item.Qty; 
      ...

我会做（也使用LINQ简化）：

public void CheckProductExistThenAddToCart(CartItem item) 
{ 
    if (CartItems.Count == 0) 
    { 
     AddToCart(item); 
     return; 
    } 

    CartItem oldItem = CartItems.FirstOrDefault(ci => ci.ProductId == item.ProductId); 
    if(oldItem == null) 
     AddToCart(item); 
    else 
     oldItem.Qty += item.Qty; 
}

来源

2013-08-31 23:14:39

其实第一个'if'语句与'FirstOrDefault'是多余的。但是，我保留它以显示缺少的返回语句，隐藏错误的原因。 –

使方法更少的代码行

回答

相关问题