3
\$\begingroup\$

I am trying to create a class to test my understanding, and it takes an ArrayList as parameter. The decision of choosing an ArrayList is because it does not have a fixed size and I can add more elements to it later.

I came out with this implementation:

public class Character {

    private String name;
    private char gender;
    private List<String> inventory = new ArrayList<String>();

    public Character(String name, char gender, List<String> inventory) {
        this.name = name;
        this.gender = gender;
        this.inventory.addAll(inventory); // Is this a good idea?
    }

    public String getName() {
        return name;
    }

    public char getGender() {
        return gender;
    }

    public List<String> getInventory() {
        return inventory;
    }

}

And in the main class:

public class Main {

    public static void main(String[] args) {

        Character coolKid = new Character("Cool Kid", 'f',
                                          Arrays.asList("just", "testing"));

        System.out.println(coolKid.getInventory());
        // returns [just, testing]

    }
}

My main concern is "dynamically" adding elements to the ArrayList when instantiating, as well as using addAll in the constructor, and List as the type for the inventory argument in the constructor.

Is there a better way to accomplish this?

\$\endgroup\$
4
\$\begingroup\$

This is a very straightforward implementation. Not much to improve.

Especially copying the list passed in from the caller is a good safety measure, as it makes sure that, if the caller later modifies his list, this doesn't affect the Character's inventory. It's always a good idea that changing an instance's state is done by using its methods (so it can adjust itself appropriately, e.g. drop some items if the inventory becomes too heavy).

A few ideas to think about:

Similarly to the constructor, you can make the getInventory() method safe against a caller's attempts to modify the list, by encapsulating it in a read-only wrapper:

return Collections.unmodifiableList(inventory);

Then the caller can read the list, but not change it.

You might want to implement some more inventory methods to add, remove or find inventory items.

There also is a programming style where you wouldn't implement add, remove or find methods for the inventory, and have the getInventory() method return the internal list, allowing the caller to directly modify that list. But that breaks encapsulation and generally leads to quality problems, so don't do it that way.

Probably, the ordering of items in the inventory doesn't matter, so you could use a Set instead of a List (but then you can't have multiple instances of the same item).

And you could change the constructor parameter inventory from List to Collection, allowing the caller to pass the items as List or Set, whatever suits him better.

Maybe, when your project evolves, you'll find that inventory items could have their own properties and behaviour, so you might want to change from List<String> to List<InventoryItem> early.

\$\endgroup\$
  • \$\begingroup\$ Thank you for you answer! I am quite new to OOP, and specially to Java. What is the benefit of encapsulating the inventory getter? Also, today i realized i could prefer a dictionary, or Python's dictionary equivalent in Java, for handling inventory, since it would be a good idea to classify items by type. I am going to definitely change the type to Collection, it was a doubt i had. Still confused with types. Excellent, i am sure i will create an InventoryItem class as well. \$\endgroup\$ – ManuAlvarado22 Feb 26 '18 at 10:11
  • \$\begingroup\$ @ManuAlvarado22 I added a few words on encapsulation. \$\endgroup\$ – Ralf Kleberhoff Feb 26 '18 at 11:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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