Return to Answer

Thread Safety:

I touched on a few things in some of the other points, but I haven't addressed the main implementation as a whole. The current implementation is not thread safe. While you are using a ConcurrentHashMap to store the quantities, pickProduct() and restockProduct() both make multiple calls to that instance. ConcurrentHashMap ensures that one thread can't call put() while another thread calls get(). However, it does not prevent the following:

1. T1: map.contains(x);
2. T2: map.remove(x);
3. T1: map.get(x);

In this case, the class that has the reference to the Map must ensure that multiple sequential calls happen in a logical block.

public Inventory {
  private final Object _lock = new Object();
  private final Map<Product, Integer> _quantities = new HashMap<>();
  private final Map<Integer, Product> _idToProduct = new HashMap<>();

  public pickProduct(int id, int quantity) {
    synchronize(_lock) {
      if (!idToProduct.containsKey(id)) {
        throw new NoProductException(id); // the UI class would catch this and decide how to tell the user something bad happened
      }  else if (quantity <= 0)
        throw new IllegalArgumentException(quantity);
      }
      Product product = _idToProduct.get(id);
      int currentCount = _quantities.get(product); // Java will unbox the value for you
      if (currentCount < quantity) {
        throw InsufficentQuantityException(id, quantity);
      }
      _quantities.put(product, currentCount - quantity); 
    }
  }
}

Once you follow the similar pattern for wrapping your public methods in synchronize blocks, this will ensure that one thread can not partially execute pickProduct() while another thread is trying to restockProduct() the same product.

In general, each class should do one thing and do it well. The Inventory class should be in charge of tracking what products it contains and how many items exist for each product. It might have the following interface:

public interface Inventory {
  void addProduct(Product product);
  void removeProduct(Product product);
  void stockProduct(Product product, int count);
  void pickProduct(Product product, int count);
  List<Product> allProducts();
  int countOf(Product product);
}

There might be a few other methods to make the interface easier to work with, such as one that can add a product with an initial count or add a collection of products, but the basics are there.

There would then be an independent class that would be in charge of parsing your file format and producing the a Map<Product, Integer> that could be then added to the Inventory. This way, if you wanted to change your application so that the user can add a new product, they can do that directly instead of editing a text file and then restarting the application.

Finally, there would a third class that handles interacting with the user. As it interacts with the Inventory instance, it decides how these changes should be displayed to the user. By separating the UI from the business logic you can change from a console front end to a GUI or web front end without needing to make any changes to the code Inventory class. Your main() could be as simple as simple as creating am Inventory instance and passing it to the UI class and telling the UI class to start executing.

Separation of Concerns:

Inventory knows about anything this code will ever do and this prevents it from being used in other locations.

• buildInventory() is the only way to add items to the inventory. In tern, this means this code can only be used on a computer that:

1. Runs Windows
2. Has the directory path \dev\eclipse_workspace\Warehouse\src\.
3. That directory contains a files with a specific custom format.

• All of the other methods write directly to stdout. This means the code can only be used as a console application or it will write a bunch of output that might not make sense for a GUI application.
• There are very few operations that can be done with the information in the inventory and there is no way to do anything else.

I'm fairly confident that you are just starting out, so all of these things are not that surprising. It is more important to see how things can be done differently to allow you to reuse the code you have already written in different ways. While the following suggestions will make the project more complex, I hope you can see how it provides more flexibility.

• Keep all the business logic in Inventory.
• Extract all the output to the user to a different class that uses an Inventory instance.
• Extract the code to parse the file and produce the initial mapping of products to quantities. Then pass that mapping in as an argument to the Inventory constructor.