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

Question

我有一个名为 conveyorBelt 的 ArrayList，它存储已被拣选并放置在传送带上的订单。我有另一个名为 readyCollected 的 ArrayList，其中包含客户可以收集的订单列表。

我试图用我创建的方法做的是，当输入 ordNum 时，如果订单已准备好由客户收集，它将返回 true（从而从 ordNum 中删除收集的订单） >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;
      }
  }

Answer 1

我有点困惑，因为您根本没有使用 ordNum 。

如果您想确认代码的运行并总体上提高所编写内容的可靠性，您应该查看 单元测试以及可用于此的 Java 框架。

Answer 2

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

相反，我建议使用将订单号与特定订单关联起来的 Map。这可以更自然地编码您想要的内容 - 每个订单号都是与订单关联的键。 Map，尤其是 HashMap，具有非常快的查找速度（预期恒定时间），并且无论有多少个键，都使用（大致）相同的空间量。此外，从 HashMap 中插入或删除元素的时间预计为常数时间，速度非常快。

至于您的特定代码，我同意 Brian Agnew 的观点，您可能想为其编写一些单元测试，并找出为什么不使用 ordNUm 参数。也就是说，我建议在执行此操作之前重新设计系统以使用 HashMap 而不是 ArrayList；时间和代码复杂性的节省将真正得到回报。

Answer 3

根据您的描述，为什么这还不够：

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

为什么传送带 ArrayList 甚至需要检查？

Answer 4

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

除此之外，任何人都可以用您发布的代码给出的最佳答案是“也许”。您的逻辑看起来当然是正确的并且与您所描述的内容相关，但它是否正在做它应该做的事情完全取决于您在其他地方的实现。

作为一般指针（在这种情况下可能适用也可能不适用），您应该确保您的代码处理边缘情况和不正确的值。因此，例如，如果 readyCollected.remove(b); 返回 false，您可能希望标记出错误，因为这表明 b 不在要删除的列表中。

正如已经指出的，请查看使用 JUnit 进行此类事情的单元测试。它很容易使用，并且编写完整的单元测试是一个非常好的习惯。