Once And Only Once

Joseph Bergin
Pace University
jbergin@pace.edu
http://csis.pace.edu/~bergin

The following problem occurred in developing the DPS project inventory class. The question arose (with some vehemence) of how best to decrement the inventory records so that they can be guaranteed to never go negative. That is to say, you can never have a negative amount on hand.

Bergin wanted a solution like the following:

public void decrement(int amount)
{	quantity -= amount;
	if(quantity < 0)
	{	quantity = 0;
	}
}

Grossman objected, rightly, as this will only mask an error in logic elsewhere in the program. Bergin's goal, however, was to make sure that there was a single point in the program (the Inventory class) in which to implement the policy. In the published code, this was changed as follows.

public void decrement(int amount) throws InventoryException
{	quantity -= amount;
	checkQuantity();
	if(quantity < 0)
	{	quantity = 0;
	}
}

Where checkQuantity is defined as:

private void checkQuantity() throws InventoryException
{	if(quantity < 0)
	{	throw new InventoryException();
	}
}

Note that InventoryException is a new subclass of RuntimeException, and as such does not need to be mentioned in a throws clause (thought I did for documentation purposes) and does not need to be trapped in a try block. You may trap it, but need not. The philosophy of a runtime exception is that it should never occur in a valid program, so the programmer should not have to trap every possible occurrence. The set quantity method of the class was also modified to call checkQuantity. As these are the only two methods that change quantity (though the constructor has a bug in this regard) we are guarded against setting a negative quantity by errors other places in the program. Note that the negative check and the set to zero is necessary to maintain the invariant. We want the invariant to hold even in an incorrect program.

There is still a problem, however. We have violated the OnceAndOnlyOnce principle of good OO programming. Everywhere in the program that inventory quantities might be changed, the programmer of that part must do the math to be sure that a negative quantity won't result. This could result in repeated code in a large number of places, any of which could be subtly buggy.

The problem is a bit deeper than this, however, if the exception is indeed thrown, we don't have information in the program about how much the inventory was decreased by in the correction that prevented inventory from going negative. This will be corrected below after we look a bit harder at the problem.

Note that it should be an invariant of the program as a whole that inventory quantities should never be negative. The question is how to implement this invariant. I maintain that it should be implemented as an invariant of the Inventory class itself, and that this invariant should be implemented in such a way as to use the OnceAndOnlyOnce principle. We don't do that in the above code as suggested above.

If you look at how we decremented inventory in practice it will show the problem better. In the InventoryTest class we said something like the following:

int earlyQuantity = inv.quantity();
int quantityShipped = Math.min(orderQuantity, earlyQuantity);
Order order = ...
Orderprocessor.orderProcessor.generateInvoice(order);
int lateQuantity = inv.quantity();
assert(...

Then in the InventoryManager class's available method we actually decremented the inventory as part of the call to generate the invoice:

int orderQuantity = order.quantity();
int onHand = recordFromInventory.quantity();
int quantityShipped = Math.min(orderQuantity, onHand);
recordFromInventory.decrement(quantityShipped);
return new OrderItem(...

It is in this repeating of the min calculation that we see the problem. This kind of code is replicated possibly throughout the program. We would like to avoid this.

My suggestion is to move this calculation inside the Inventory class. This was done in effect by the above code, but it didn't provide enough information to use the decrement method well and wisely.

Here is a better version:

public int decrement(int amount)
{	amount = Math.min(amount, quantity);
	quantity -= amount;
	return amount;
}

Note that it returns the amount by which it actually decremented inventory, which never leaves it negative. Its use is different, however. In InventoryManager.available, we will say instead:

int quantityShipped = recordFromInventory.decrement(order.quantity());
// If you need backorderQuantity it is order.quantity() - quantityShipped. 
return new OrderItem(...

We don't need to get the onHand. We are told how how many it was decremented by (quantityShipped) and we are guaranteed it doesn't leave onHand negative.

 

Still, the error in the constructor needs to be fixed. Do that. Then, the constructor establishes the invariant. All methods maintain it. Therefore the invariant of the class guarantees the invariant of the program. QED.

Note that decrement is perhaps not the best name with this solution: perhaps request instead.

Last Updated: November 22, 2001