3

Suppose I have a Map which is a private member variable of my class A.

In the same class I create an instance of class B, call it objB, by passing the map to B's constructor.

Obviously this is wrong, because I have leaked a private field to an object of a different class. That class might later on decide to modify the map passed to it, which would ruin everything.

Is it preferable to 1) pass is using Collections.unmodifiableMap or maybe 2) copying it? If there's another, better approach, I'd be glad to see it.

In case the map is modified in class A, objB should know the new map immediately. And this means I'd have to update the map in objB every time I modify it (but not if I pass it ad unmodifiable list).

I think option 2 is better, because in 1) we pass an unmodifiable collection to B, and B assumes the collections passed to it are modifiable.

class A {
    Map<String, Integer> map;

    public A() {
        map = new HashMap<String, Integer>();
        B objB = new B(map); //option 1
        B objB = new B(Collections.unmodifiableMap(map)); //2
        B objB = new B(new HashMap(map)); //3
    }
}

class B{
    Map<String, Integer> map;

    public B(Map<String, Integer> map) {
        this.map = map;
    }

    //use data from the map to do stuff
}
3
  • Why don't you go ahead and post a code example. It's hard to figure out exactly what's happening from your verbal description. May 21, 2016 at 18:04
  • This probably depends on what A and B are doing, and what their contracts are. For instance, if B is documented as taking a map in its constructor and never ever modifying it, and you trust that documentation, this is harmless. If B's job is to do something that should never involve modifying the map (e.g., searching for stuff it), then this might also be harmless. In fact, does the map even have to be a constructor argument? I think we just need more detail on why this situation exists before we can tell whether or not it's a bad situation.
    – Ixrec
    May 21, 2016 at 18:11
  • @Ixrec it doesn't have to be a constructor argument. B objects simply need the access to A's map. Usually when a class needs some data, you pass it to constructor, that's why I did it. May 21, 2016 at 18:13

3 Answers 3

4

In case the map is modified in class A, objB should know the new map immediately.

If by know immediately you mean assume the same state immediately without doing anything about it then B should just hold a reference to the map. You can protect it with unmodifiableMap. This approach is not multi threaded friendly since A is still free to mutate the map when B might be traversing it.

If by know immediately you mean deal with the change in state in some way then use the observer pattern. B is an observer of the state of A. When A changes state it should notify all observers. This way A is not forced to expose its reference to the map to keep B up to date.

It would probably be a good idea to read up on immutability and defensive coping.

5
  • Regarding the observer pattern - when a change occurs, I need to inform class B about it, and this would mean passing it a new copy of the modified map, right? May 21, 2016 at 20:02
  • 1
    I've seen it done where the observed object passes the new state (the linked wikipedia article examples do this). I've seen implementations that have the observers interrogate the observed object (say using getters). I prefer the first but that's just because I like real encapsulation. See tell, don't ask. May 22, 2016 at 0:42
  • Immutable objects are by definition thread-safe. I do not understand why you would state "This approach is not multi threaded friendly."
    – YoYo
    May 22, 2016 at 0:44
  • 3
    @YoYo, The first paragraph is about sharing a reference. Sure once it's wrapped in the unmodifableMap interface the map is not mutable by B but it remains mutable by A. Thus the map could change while B is traversing it in a different thread. It's only immutable if nothing can mutate it. May 22, 2016 at 3:10
  • Ok ... Point to be made is that unmodifiableMap is not immutable at all.
    – YoYo
    May 23, 2016 at 23:53
0

If B needs to search inside A's data, then it should use A's public interface to do so.

1
  • 2
    This isn't necesarily a wrong answer, but you'll probably get more downvotes if you don't elaborate. Here we prefer answers which explain why, not just what. Also, I can see a problem with your answer: what if B is the way A solves a computation, i.e. B is an implementation detail? In this case, B cannot simply call A, because it would result in endless recursion!
    – Andres F.
    May 22, 2016 at 15:01
0

I think your question points to a lack of understanding what your domain-problem is you are trying to solve.

  • If you have two objects A and B and B wants to know something from A, it should simply ask.

  • If B wants to mutate As state, it should ask A to do so.

You only need a connection between both objects, e.g. via composition or as a parameter when calling B to do something.

Obviously this is wrong

Yes. It breaks encapsulation.

If there's another, better approach, I'd be glad to see it.

There is: e.g. B.doFreakyStuffWith(A)

2
  • What is the context of this B.doFreakyStuffWith(A) call? Is it in some method of A, B, or some unmentioned C? May 22, 2016 at 17:28
  • Since we have no real context, it depends: it could be a call in a method of B or it could be used in a mediator pattern context May 22, 2016 at 19:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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