我用正确的方式写这个方法吗？

问题描述：

我有一个叫做conveyorBelt的ArrayList，它存储已经拾取并放置在传送带上的订单。我还有另一个ArrayList，名为readyCollected，其中包含客户可以收集的订单列表。我用正确的方式写这个方法吗？

我想要做的与我创建的方法是当输入ordNum，它返回true，如果订单已准备好由客户收集（从而从readyCollected删除收集的订单）。如果订单还没有被选中，那么它返回false。

我想知道这是写的方法，以正确的方式...

public boolean collectedOrder(int ordNum) 
    { 
     int index = 0; 
     Basket b = new Basket(index); 
     if(conveyorBelt.isEmpty()) { 
      return false; 
     } 
     else { 
      readyCollected.remove(b); 
      return true; 
     } 
    }

我们看不到很多正在发生的事情。什么是篮子？它的平等方法是否明确？什么是类型readyCollected？ – 2011-01-31 21:05:09

您没有使用`ordNum`，所以显然有些问题。另外，b总是'Basket（0）`。也许它应该是`Basket（ordNum）`？我们需要看到更多的代码！ – 2011-01-31 21:07:56

@Andrew White，Basket是持有产品集合的类（这是用于arrayList的）。 – Mayfield 2011-01-31 21:14:30

答

我有点困惑，因为你不使用ordNum可言。

如果你想确认你的代码的操作并且通常增加你写的东西的可靠性，你应该检查出unit testing以及可用于此的Java框架。

谢谢，Amir Rachum指出我根本不使用ordNum。但是我写的东西有什么不对吗？ – Mayfield 2011-01-31 21:24:19

答

您可以使用ArrayList来解决这个问题，但我认为这是从根本上考虑问题的错误方法。一个ArrayList适用于存储完整的数据序列，而且没有间隙，因此您只可能在最后添加或删除元素。删除其他位置的元素效率不高，如果您在高索引处只有一个值，那么您将浪费大量空间填充空值的所有较低位置。

相反，我建议使用Map将订单号与特定订单相关联。这更自然地编码你想要的 - 每个订单号码都是与订单相关的关键。 Map s，特别是HashMap s，具有非常快速的查找（预期的恒定时间）并且使用（大致）相同的空间量，而不管有多少个密钥。此外，插入或从HashMap中删除元素的时间预计是恒定的，这是非常快的。

至于你的特定代码，我同意Brian Agnew在这个问题上，你可能想为它编写一些单元测试，并找出你为什么不使用ordNUm参数。也就是说，我建议在重做系统之前使用HashMap而不是ArrayList;节省的时间和代码的复杂性将真正得到回报。

答

根据您的描述，为什么不是这样足够：

public boolean collectedOrder(int ordNum) {   
     return (readyCollected.remove(ordNum) != null); 
    }

为什么conveyorBelt ArrayList中甚至需要进行检查？

答

正如已经指出的那样，您很可能需要使用ordNum。

除此之外，任何人都可以用你发布的代码给出的最佳答案是“也许”。你的逻辑看起来是正确的，并与你所描述的内容联系在一起，但它是否正在做它应该完全取决于你在其他地方的实现。作为一般指针（在这种情况下可能适用，也可能不适用），你应该确保你的代码处理边缘情况和不正确的值。因此，如果readyCollected.remove(b);例如返回false，您可能想要标记出错，因为这表示b不在要移除的列表中。

正如已经指出的那样，看看使用JUnit这种类型的东西的单元测试。这很容易使用，编写彻底的单元测试是一个很好的习惯。

我用正确的方式写这个方法吗？

相关推荐