4

I am building a WMS with the following basic requirements:

  1. Each Item can be in multiple Locations.
  2. A Location can contain multiple Items.
  3. For each Location / item combination, I must be able to track: QtyOnHand, QtyReserved, and QtyAvailable.
  4. One Location can contain another Location.

The basic required behaviour of a Location, is:

  1. Adding/Removing an Item with a Quantity.
  2. Adding/Removing a nested Location.
  3. Tracking Quantities for each Item within the Location.

I am attempting to write the code while staying faithful to OOP design principles, namely:

  1. Designing class interfaces to reflect behaviour rather than data.
  2. Avoiding Getter/Setters.
  3. Tell, don't Ask, as much as possible.

With that in mind, here is the basic code for a Location:

public class Location{ private final Map<InventoryItem, Integer> items = new HashMap<>(); private final Set<Location> locations = new HashSet<>(); public void add(InventoryItem item, int quantity) { if (items.containsKey(item)) { InventoryQuantity inventoryQuantity = items.get(item); items.put(item, inventoryQuantity.addQuantityOnHand(quantity)); } else { items.put(item, quantity); } } public void add(Location location) { locations.add(location); } public void remove(InventoryItem item, int quantity) { if (items.keySet().contains(item)) { if ((items.get(item) - quantity) == 0) { items.remove(item); } else { items.put(item, items.get(item) - quantity); } } } public void remove(Location location) { locators.remove(location); } }

So I now that I have the basic behaviour for Adding/Removing, I am having difficulty figuring out what would be the best way to track quantity. I could create an InventoryQuantity class, as follows:

 public class InventoryQuantity { private InventoryItem item; private int quantityOnHand; private int quantityReserved; private int quantityAvailable; public InventoryItem getItem() { return item; } public void setItem(InventoryItem item) { this.item = item; } public int getQuantityOnHand() { return quantityOnHand; } public void setQuantityOnHand(int quantityOnHand) { this.quantityOnHand = quantityOnHand; } public int getQuantityReserved() { return quantityReserved; } public void setQuantityReserved(int quantityReserved) { this.quantityReserved = quantityReserved; } public int getQuantityAvailable() { return quantityAvailable; } public void setQuantityAvailable(int quantityAvailable) { this.quantityAvailable = quantityAvailable; } }

and store one InventoryQuantity for each InventoryItem in Location, like so:

private final Map<InventoryItem, InventoryQuantity> items = new HashMap<>();

My questions/issues are as follows:

  1. InventoryQuantity does not seem to be a real world object with a defined behavior; rather, it seems to be a data structure with the sole purpose of storing quantity data (as evidenced by the existance of Getter/Setters); is there a better way?
  2. Is asking a location to getQuantityOnHandForItem(item) a violation of Tell, don't Ask? On the other hand, there does not seem to be a better way of retreiving data...

I actually had some more questions, but this is what I remember at the moment. If anyone can help me understand/apply the principles effectively, I would greatly appreciate it!

Improve this question
14
  • 1
    I'm a bit confused by a Location containing a set of locations. What is going on there? Why is Locations a recursive structure? What purpose does that serve? And what does a "locater" do? Commented Feb 15, 2017 at 17:53
  • @Robert Harvey that was actually supposed to be location, not locator; edited accordingly. As to your question, the reason I would nest Locations is for locations within locations, such as a shelf in a rack in an aisle in warehouse; such that one location can contain others. Commented Feb 15, 2017 at 18:07
  • I would favor something a bit more flat and explicit, like shelf, rack, aisle and custom fields in the Location object. It's going to make your life much simpler. Commented Feb 15, 2017 at 18:14
  • Your "remove" Method has some unexpected behavior. It doesn't check if I'm removing more items than I have on stock, so I can end up with negative amounts of stuff. Commented Feb 15, 2017 at 18:23
  • @Tsar there is thinking in Inventory Management that says to allow for that to happen Commented Feb 15, 2017 at 19:05

1 Answer 1

Reset to default
2

Would it make sense in your system to have a class like this:

public class QuantifiedInventoryItem extends InventoryItem { private int quantityOnHand; private int quantityReserved; private int quantityAvailable; /* getters and setters for fields above, if necessary */ }

I agree that it seems a little weird to have an object like the InventoryQuantity as you have it above. It has no behaviour, and does not map to a real-world object.

A quantified InventoryItem can map to a real-world object (well, it maps better than InventoryQuantity): an inventory item with known quantities, maybe even at specific locations.

As far as:

Is asking a location to getQuantityOnHandForItem(item) a violation of Tell, don't Ask?

No, I don't think that's a violation. I think "Tell, Don't Ask" refers more to situations where you have complex logic that depends on the internal state of an object, and that state is accessed via many getters. The pricinciple refers to moving that logic/behaviour into the object, since that's where the data that drives it already resides.

In the case of getQuantityOnHandForItem, you're not asking about specific behaviour driven by this function, so the simple existence of the function is probably not a problem. If you had a set of logic statements that constantly called a Location's getters, it might be worth considering refactoring that logic to a new function within the Location.

See: https://martinfowler.com/bliki/TellDontAsk.html

Improve this answer
16
  • Yes, I figured the answer to TDA would be along those lines, although the name 'TDA' would at first glance suggest 'always tell, never ask'... did the originators of TDA have in mind what you said? Commented Feb 15, 2017 at 17:41
  • 2
    @IntelliData You can also read about Command Query Separation. Note that it is kind of the opposite of TDA, so both approaches have their pros and cons. If you notice something that is purely a "query" (it returns information, and does not modify the state, and it is not being used as part of a bigger command that modifies state), it is not a violation of TDA's rule about "getter". Commented Feb 15, 2017 at 17:44
  • 1
    I'm kinda with rwong here. Tell, Don't Ask is a guideline I follow when I'm writing a class that has behaviors. If I'm just doing CRUD operations, I don't even think about Tell, Don't Ask. Commented Feb 15, 2017 at 17:45
  • @Robert Harvey one of my questions actually was whether it is at all OK to have a class with only getter/setters and no behavior. I just read an article from Martin Fowler, where he says it's a code smell :(. Commented Feb 15, 2017 at 18:00
  • Data Transfer Objects are used all the time, in real, working, production applications. Fowler also says that DTO's have their appropriate uses. Commented Feb 15, 2017 at 18:02

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.