0

Say I have the following code snippet to create colored vegetables for a small random game I'm making to practice separating object properties out of object classes:

    List<Vegetable> vegList = new ArrayList<Vegetable>();

    Map<MyProperty, Object> propertyList = new HashMap<MyProperty, Object>();

    propertyList.put(MyProperty.COLOR, "#0000BB");
    propertyList.put(MyProperty.TYPE, MyType.VEGETABLE);
    propertyList.put(MyProperty.COMMONNAME, "Potato");

    vegList.add(new Vegetable("Maisie", propertyList));

    propertyList.put(MyProperty.COLOR, "#00FF00");
    propertyList.put(MyProperty.COMMONNAME, "Poisonous Potato");

    vegList.add(new Vegetable("Horror", propertyList));

I realized while doing this (making my own example from Head First OOA&D, basically) I have no idea why changing propertyList the second time doesn't affect the values previously set within Maisie.

I followed the structure provided by the book, but the first time around I was creating a new HashMap for each individual Vegetable object, before adding it to the list. The book shows that's unnecessary but doesn't go into why.

All I can see is the interpreter is making a choice to create a new instance of the hashmap when it's specified in the Vegetable constructor the second time around. But why?

How does it know that I'd rather have a different HashMap in there, rather than reusing the first object and .put() changing its values for both Vegetables?


Second related question is.... should I want to actually have 2 vegetables share the exact same list of properties (the same HashMap object), how would I do that? And should this actually be a horrible idea... why? How would wanting this show I just don't know what I'm doing?

My understanding hits a wall beyond "it has to do with object references".

Thanks for helping me clear this up.


Vegetable class as requested:

public class Vegetable {
    public VegetableSpec characteristics;
    public String name;

    public Vegetable(String name, Map<MyProperty, Object> propertyList) {
        this.name = name;
        this.characteristics = new VegetableSpec(propertyList);
    }

    public void display() {
        System.out.printf("My name is %s!\n", this.name);
        for (Entry<MyProperty, Object> entry : characteristics.properties.entrySet()) {
            System.out.printf("key: %s, val: %s\n", entry.getKey().toString(), entry.getValue().toString());
        }
    }
}

... which made me look at VegetableSpec again (I put it in because the book used a separate Spec class, but I didn't understand why it was necessary beyond adding search capabilities; now I think I see it does 2 things, one is defensive copying!):

public class VegetableSpec {
    Map<MyProperty, Object> properties;

    public VegetableSpec(Map<MyProperty, Object> properties) {
        if (properties == null) {
            // return a null = bad way to signal a problem
            this.properties = new HashMap();
        } else {
            // correction above makes it clear this isn't redundant
            this.properties = new HashMap(properties);
        }

    }

}
Gee
  • 59
  • 2
  • 9
  • 1
    Post the code for `Vegetable`. Perhaps its constructor does a defensive copy of the map? – Andrew S Jan 24 '18 at 15:58
  • I think I understand what's meant by defensive copying (which happens in VegetableSpec I suppose). But I am not at all certain of the logic in the constructor, could you walk me through that one perhaps? As in... why would properties ever equal null since we're always passing a map in (unless the map was badly created), or the constructor itself would fail?? – Gee Jan 24 '18 at 16:30
  • Thinking more I think my constructor is wrong now? It should always create a new HashMap with the values it receives and isn't supposed to keep a null if the map has a problem... – Gee Jan 24 '18 at 16:37
  • 1
    `VegetableSpec`'s constructor is just being cautious just in case the map is null. In your example that would never happen. I agree with your other comment - it would be better to create an empty map rather than allowing it be null. That would simplify other methods which wouldn't have to check for null. – Andrew S Jan 24 '18 at 17:12

1 Answers1

1

It sounds like the constructor for Vegetable is making a defensive copy. It is generally a good idea to do this to prevent anyone from changing an object in ways the designer of the object does not want. You should (nearly) always be making defensive copies.

I want to actually have 2 vegetables share the exact same list of properties (the same HashMap object), how would I do that?

Pass the same hash map in, and ignore the fact that it makes a defensive copy, should not matter to you as a consumer.

Michael Lloyd Lee mlk
  • 14,561
  • 3
  • 44
  • 81
  • Ok, I think I actually understand what was going on in VegetableSpec now, thanks for the link! – Gee Jan 24 '18 at 17:49