Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I've recently had doubts about some design decisions that I frequently make, so this time I decided to take some time and try to analyze it to find the best solution. I will describe a simple scenario where the design decision can be easily understood. :)

Suppose we are developing a game in Java where players can have any number of many different types of items.

/** Class that represent an item type existing in the game */
// you may prefer to call this ItemType,
// but for me appending the word "Type" is not necessary
class Item {
    private String descrip;
    private Image icon;
}

In this situation I need to keep track of how many of each item the player has in his bag, so I keep a collection of them, in each element of which I have item type and quantity. Additionally one can think of other attributes that belong to that relation (perhaps a max capacity), but the question is: how do I model this scenario in general but with a practical implementation in Java? Here are two approaches I've come up with:

Using a List

class Slot {
    private Item item;
    private int quantity;

    /** Auto generated */
    public int hashCode()
    {
        int hash = 7;
        hash = 13 * hash + (this.item != null ? this.item.hashCode() : 0);
        return hash;
    }

    /** This method is also auto generated and just compares the Item, 
        independent of quantity */
    public boolean equals(Object object) 
    {
        if (!(object instanceof Slot))
        {
            return false;
        }
        Slot other = (Slot) object;
        if ((this.item == null && other.item != null) 
             || (this.item != null && !this.item.equals(other.item)))
        {
            return false;
        }

        return true;
    }
}

The above class relates one item with a quantity, and a collection of Slots is the primary attribute of the Inventory class. The disadvantages of this appear as soon I design the Inventory interface to not expose Slot to the client (making Slot private or hiding it in some other way): in creating a simple interface but providing access to all the standard collection methods. In my humble opinion there is nothing wrong with this:

/** Class that represent the bag that holds the player's items */
class Inventory {
   // this collection holds only one element for each type of item
   private ArrayList<Slot> slots; 

    public Inventory()
    {
        slots = new ArrayList<Slot>(10);
    }

    public void add(Item item, int quantity)
    {
        Slot slot = new Slot(item, quantity);
        // if we don't already have a Slot for the added Item, add it
        if (!slots.contains(slot))
        {
            slots.addElement(slot);
        }
        // if we already have a Slot for the added Item, increase the quantity
        else
        {
            Slot _slot = slots.elementAt(slots.indexOf(slot));
            _slot.addQuantity(quantity);
        }
    }

    public void subtract(Item item, int quantity)
    {
        Slot slot = new Slot(item, 0); 
        int index = slots.indexOf(slot);

        // only if we actually have this item
        if (index != -1) 
        {
            Slot _slot = (Slot) slots.elementAt(index);
            _slot.quantity -= quantity;

            // if we have fewer of the Item than the number to remove, 
            // remove the slot from the collection
            if (_slot.quantity <= 0)
            {
                slots.removeElementAt(index);
            }
        }
    }

    public void remove(Item item)
    {
        Slot slot = new Slot(item,0);
        slots.removeElement(slot);
    }

    public boolean hasItem(Item item)
    {
        Slot slot = new Slot(item,0);
        return slots.contains(slot);
    }
}

As you see, we have to jump through some hoops to implement certain collection methods: in hasItem we have to create a Slot element just for that operation. In the add and subtract methods, we have to create the Slot object just to check if it's there. I wonder if this really necessary. I suppose that it is if we desire to provide an interface to the standard collection methods. Perhaps we can move all this logic to another class (for example: Slots), exposing a cleaner interface that lets us search by item without the need of creating a Slot, but we are just moving the same problem to another class. We will face the same problem there, besides the fact that Inventory is basically a set of Slots, we're really just creating another class to be exactly the same thing, which doesn't make a lot of sense to me.

Would it be more correct to change the interface of this class to work directly with Slot class instead? This implies moving from composition (having Slots created within Inventory) to an aggregation (having Slot creation outside of Inventory). The result might look like:

/** Class that represent the bag that holds the player's items */
class Inventory {
   private ArrayList<Slot> slots;

    public Inventory()
    {
        slots = new ArrayList<Slot>(10);
    }

    public void add(Slot slot)
    {
        if (!slots.contains(slot))
        {
            slots.addElement(slot);
        }
        else
        {
            Slot _slot = slots.elementAt(slots.indexOf(slot));
            _slot.addQuantity(slot.getQuantity());
        }
    }

    public void subtract(Slot slot, int quantity) // how would the client get the Slot?
    {
        int index = slots.indexOf(slot);

        // only if we actually have this item
        if (index != -1) 
        {
            Slot _slot = (Slot) slots.elementAt(index);
            _slot.quantity -= quantity;

            // if we have fewer of the Item than the number to remove, 
            // remove the slot from the collection
            if (_slot.quantity <= 0)
            {
                slots.removeElementAt(index);
            }
        }
    }

    public void remove(Slot slot) // again, how would the client get the Slot?
    {
        slots.removeElement(slot);
    }

    // Hard to change a parameter type here. This strongly suggest there is
    // a flaw in the design. I only keep this here to communicate the observation
    public boolean hasItem(Item item) 
    {
        Slot slot = new Slot(item, 0); // here we have no choice but to create it
        return slots.contains(slot);
    }
}

But again, is this convenient? I don't often put this approach into practice, so I'm not sure.

Using a lookup

Another approach is to use something like a Hashtable with Item objects as keys quantities as values, so only quantity is associated with Items, without the need for a Slot class. An Inventory implementation using Hashtable might look like:

public class Inventory
{
    private Hashtable items;

    public Inventory()
    {
        items = new Hashtable(10);
    }

    public void add(Item item, int amount)
    {
        int stock = amount;
        if (items.containsKey(item))
        {
            stock += ((Integer) items.get(item)).intValue();
        }
        items.put(item, new Integer(stock));
    }

    public void subtract(Item item, int amount)
    {
        if (items.containsKey(item))
        {
            int stock = ((Integer) items.get(item)).intValue();
            stock -= amount;
            if (stock > 0)
            {
                items.put(item, new Integer(stock));
            }
            else
            {
                items.remove(item);
            }
        }
    }

    public void remove(Item item)
    {
        this.items.remove(item);
    }

    public boolean hasItem(Item item)
    {
        return this.items.containsKey(item);
    }
}

To me this seems much better than using a List because there is no need of the Slot class, and all interactions with the Item collection are much simpler, but I'm not sure if I'm right.

The same type of problem is also evident in this scenario: Suppose that we are implementing an order application for a pizza shop; every order consists of types of pizzas and quantities.

/** Type of pizza */
class Pizza {
    private String descrip;
    private Image icon;
}

/** Customer request for pizza */
class Request {
    // for each type of pizza we have to keep a count of how many the customer ordered
}

/** A pizza order */
class Order {
    private Pizza pizza;
    private int   amount;
    privete BigDecimal price;
}

Knowing how to solve the first problem, I have a pattern to use for this similar situation; so the question is, what solution is best?

I try to convince myself that I'm not the only person with this concern, that ordinary people like me face this problem on a regular basis, so I hope to find how other people solve this problem. On the other hand, I know that this question is quite big and maybe a little elaborate, so maybe I need to split it into several more specific questions. Maybe the answer to this problem is simple I'm over-complicating things, or even worse, my doubts about how to handle this situation arise because I really don't understand some basic design principle, like a captain on a ship using a spyglass in order to sight land but doesn't realize that his ship has been beached on solid ground that was close at the moment he draw his spyglass to look. Okay, enough cheap poetry (but it was hard to describe my feelings without it hehehe).

I apologize for the length of the question, and would very much appreciate anyone's help with this problem.

share|improve this question

migrated from stackoverflow.com Apr 10 '13 at 13:58

This question came from our site for professional and enthusiast programmers.

    
thanks rajesh, i will keep in mind that –  Victor Apr 8 '13 at 15:06
add comment

2 Answers

There's a lot to say here.

  1. Inventory's api looks like a value object (since it has no identity, on its own, I suppose that it belongs to a Player). If so, it should be immutable, returning a new instance each time you add or remove an Item.
  2. Your Item class looks like a shared identifier, thus it should override equals and hashcode and (strictly speaking, in DDD term) it should not hold any reference to the icon since it's not useful for its own invariants. You should use a domain service that take the Item and provide the proper icon instead (probably wrapping a statically defined Hashtable). However, be pragmatic with this: if you measure actual performance issues with such (formally correct) approach, keep the field there.
  3. The Slot, as it stands, cannot be considered a domain object, because it does not implement equality in a domain relevant way (it ignores the quantity). This is perfecly correct for a private class internally used by the Inventory, but should not be exposed to clients (as in the second Inventory's implementation of the first solution you proposed). This because the clients could compare for equality two Slots from two different players, obtaining unreliable results.
  4. To get rid of most boilerplate code, I would code
    • a supporting functor interface like ItemQuantityChanger (below)
    • a "functional" immutable collection, say ItemsSet, with a metod of signature ItemsSet set(Item item, ItemQuantityChanger transformation) that iterates over a simple array for the item to change. Such method would always create a new ItemsSet to return, without the Item (if the trasformation returns zero), with a new Item if the item is not found (with the quantity returned by applying the transformation to zero) or simply with the previous item coupled with the result of the trasformation.

However, concretely, I would just use an ImmutableList.Builder, since the functional design described in 4 adds few value in this case. A single if, in a method, is not that huge smell.

public interface ItemQuantityChanger {
   /**
    * Accept the current quantity of an Item and return the new one.
    */
   int applyTo(int currentQuantity);
}
share|improve this answer
2  
Thanks Giacomo! i really appreciate your answer! As soon i finish some work to do i will start to understand what you says! –  Victor Apr 8 '13 at 15:08
    
I am trying to get the picture.. perpahs you can wrote an implementation of Inventory in order to see what are you talking about. And about slot class, you mention that is not a domain object because it isn't identificable.. but what about if a add attribute to slot that maintains a reference to his Inventory "container", and the inventory also contains a reference to the player. So i can implement a equals in Slot class using the inventory reference (besides the side effect if where are planning to use hibernate later... :S)... it changes somethings, huh? –  Victor Apr 9 '13 at 14:54
    
Really?!? :-D I can't promise it soon... –  Giacomo Tesio Apr 9 '13 at 15:00
    
No I stated that Slot is not a domain object because of it's equality implementation. And BTW, it's up to the domain expert to state what is part of the domain model and what it's not. Moreover cyclic reference between objects are always a weird smell, to avoid everytime it's possible. –  Giacomo Tesio Apr 9 '13 at 15:03
    
I see, is a valid point of view. Don't worry there is no rush, a spent a lot of time until i get to this question... so i don't expect immediate answers :), with good wave all can go on! –  Victor Apr 9 '13 at 20:34
add comment

As far as I can tell, your question boils down to (in the context of your example):

  1. What kind of interface should Inventory have?

  2. How should Inventory track its Item collection?

What kind of interface should Inventory have?

Inventory is an abstraction that simplifies how the Player class deals with its items, so the interface should be defined so that Player knows nothing about the implementation of that collection. This allows you to completely change how Inventory tracks items - maybe you want to start tracking total capacity by Item weight instead of size, etc - without requiring any changes in the Player code. Besides that, managing the collection of Items is the responsibility of Inventory, so the logic to do so doesn't belong anywhere else.

Just a note: max capacity doesn't seem to belong where you described it. Ideally your Inventory would have a max capacity, say 100, and different Items would contribue a certain value to that capacity; for example, a health potion would contribute 5 units, a pair of boots would contribute 20 units, etc.

How should Inventory track its Item collection?

A List is preferable if you want to hold multiple copies of the same item, and since in this case you want only distinct items, use a lookup. Hashtable is synchronized, but naively (see one of the comments to this SO question), so HashMap would probably serve better. Of course since we should always program to interfaces rather than implementations, it would be:

Map<Item, Integer> items = new HashMap<Item, Integer>();

Another note: you don't generally need to specify initial collection capacity unless you're optimizing, and in any case, Hashtable's initial capacity is 11, and HashMap's is 16.


To get a better handle on design principles, read up on SOLID principles, and if you want to go in-depth in all aspects of software development, buy a copy of Code Complete, and read it. :-)

share|improve this answer
1  
Thanks Mike! I read the SOLID principles of Robert Martin, also read half of the book of OO Analysis and design with applications of Graddy Booch. Neither of both lectures gives a clue about how to handle this kind of specific question, although they give invaluable content and concepts. The book that give me more about design principles was the head first - design patterns. The book design pattern by the Gof was also not very very effective (in learning terms) but indeed probe to be good supporting material. I do not have Code Complete, but heard of it... i guess have to try. –  Victor Apr 11 '13 at 14:54
1  
So using a map data structure is more suited to this kind of scenario where i want to have distinc items in the underlying collection.... i guess that is the best. What you stated about item weight is a good observation, i was considering that each slot can stores a fixed size of items of those type... so for that i relate slot with capacity. The other capacity in term of weight is a very good thing to consider in inventory class, that surely will be necessary to fulfill the condition to store more items of a type... anyway.. if you have some code, i would like to see it! A lot Thanks Mike!! –  Victor Apr 11 '13 at 15:00
2  
The code is up to you, but here are some ideas. How you restrict inventory depends on the game and how realistic you want it to be. For example, in a standard RPG, a character's ability to carry a given weight comfortably could depend on their strength. You might allow a minimum of 50lbs to be carried by characters with strength of 1, with each additional point allowing 10lbs more. Armor worn and weapons held should also be taken into account, so the combination of these (armor, weapons, inventory) would be compared to the character's ability to carry whenever they want to pick up an item. –  Mike Partridge Apr 11 '13 at 19:42
    
Even then you might allow extra weight to be carried at the expense of movement ability, so for each 10lbs that a character is carrying over their strength-based max, you might slow them down by 10%. At 100lbs over they might not be able to move at all. –  Mike Partridge Apr 11 '13 at 19:48
    
To implement this system, on the Player you would track their carrying ability and current burden (sum of armor, weapons, bag), and compare them on each movement action (calculating distance reduction based on amount overweight, if any) and when trying to pick up an item. –  Mike Partridge Apr 11 '13 at 19:55
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.