Java : Duplication of methods

Question

This feels like such a basic question but this is all new for me:

I have a Person and Room class, both of which have a list of Item objects.

public class Person{
  private ArrayList<Item> items;

  public Person() {
    items = new ArrayList<>();
  }

  public void addItem(){
  ...
  }

  public void removeItem(){
  ...
  }

}    

public class Room {

  private ArrayList<Item> items;

  public Room () {
    items = new ArrayList<>();
  }

  public void addItem(){
  ...
  }

  public void removeItem(){
  ...
  }

}

The item methods e.g. addItem() are duplicated in both the Room class and the Person class which wasn't very nice. I thought about making a separate Inventory class which has a list of items and item methods and then every room and person would have an inventory.

But then I wouldn't be able to call the Item methods from a Person or Room if I use a private Inventory field.

What's the best way to stop duplication here? Thanks in advance!

You could create appropriate methods in the Person and Room classes which delegate to the inventory object. — Paul Richter
– Paul Richter, Commented Oct 16, 2014 at 21:09
Your solution seems the right way. I would say you try to implement the aggregate design pattern. Maybe the thougts help a little bit: commons.oreilly.com/wiki/index.php/… — Jordi Laforge
– Jordi Laforge, Commented Oct 16, 2014 at 21:30

5gon12eder · Accepted Answer · 2014-10-16 21:19:27Z

7

You are right. Making a separate inventory class would be a good OOP design.

I'm glad you didn't say making a parent class to Room and Person since while that would save you the duplication, Rooms and Person's aren't related so they shouldn't really be related in an Object Oriented sense either.

You can use delegation to delegate the add/remove items to your Inventory field.

public class Room {

    private Inventory inventory = new Inventory();

    public void addItem(Item item) {
        inventory.addItem(item);
    }

    public void removeItem(Item item) {
        inventory.removeItem(item);
    }
}

EDIT

Some people are proposing exposing the Inventory and then having a public add/remove methods on that person.getInventory().addItem(item). I think that would violate the Law of Demeter

edited Oct 16, 2014 at 21:19

5gon12eder

25.5k6 gold badges51 silver badges99 bronze badges

answered Oct 16, 2014 at 21:12

dkatzel

31.8k3 gold badges67 silver badges68 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

5gon12eder Over a year ago

+1. Regarding your “edit”: I think it depends on the actual domain whether this is a violation or not. “Give me your inventory and let me add / remove items to / from it” might be a semantically correct operation in some domains while in others, it won't. For starters, a good test might be to ask: Does a Person have an Inventory or does it just happen to use one internally?

mmgro27 Over a year ago

Thanks! I thought of doing this but was wondering if there were other ways. But now I think your way is probably the best.

dramzy · Accepted Answer · 2014-10-16 21:10:20Z

0

I think the inventory class is the best way to go here. You would be able to use the item methods by creating a getter for the inventory inside Personand Room.

answered Oct 16, 2014 at 21:10

dramzy

1,4291 gold badge12 silver badges26 bronze badges

5 Comments

mmgro27 Over a year ago

That didn't seem like a nice solution to me. Is it a good idea to use a getter in this way?

dramzy Over a year ago

You could use delegate methods as others suggested, but I don't see the problem with using a getter for the inventory. The inventory class will have a private ArrayList and public methods for adding and removing items.

Mark Over a year ago

By not using a getter and delegating to the Person or Room class you have better encapsulation as you are not revealing the detail of how items are stored to the caller.

dramzy Over a year ago

What details are hidden by delegating to the Person and Room classes that can not be hidden in the inventory class?

Mark Over a year ago

The fact that a Person or Room uses an Inventory to store items. The caller shouldn't care how items are stored.

Brian Deragon · Accepted Answer · 2014-10-16 21:13:59Z

0

Depending what your business domain is, you could have an abstract storage container class too that they both inherit from. The abstract storage container would have the methods on it, so you could still call them directly.

You also could give both classes an empty interface of IStorageContainer and then create a new static class with a static method that took in the first parameter of IStorageContainer.

Then you could call AddItem(thisPerson, item) and RemoveItem(thisPerson, item) but be able to reuse those two methods for both classes, using the same code and implementation.

answered Oct 16, 2014 at 21:13

Brian Deragon

2,96526 silver badges44 bronze badges

Collectives™ on Stack Overflow

Java : Duplication of methods

3 Answers 3

2 Comments

5 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

5 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related