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
}
  • 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. – Robert Harvey May 21 '16 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 '16 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. – user4205580 May 21 '16 at 18:13
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.

  • 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? – user4205580 May 21 '16 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. – candied_orange May 22 '16 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 '16 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. – candied_orange May 22 '16 at 3:10
  • Ok ... Point to be made is that unmodifiableMap is not immutable at all. – YoYo May 23 '16 at 23:53
0

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

  • 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 '16 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)

  • What is the context of this B.doFreakyStuffWith(A) call? Is it in some method of A, B, or some unmentioned C? – candied_orange May 22 '16 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 – Thomas Junk May 22 '16 at 19:18

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.