How best to avoid member implementation of class' Abstract/Interface instance variables

Question

I'm sorry if the phrasing of the question is a bit unclear but let me try to clarify below. (If anyone can word it better, feel free to edit)

I have a Map instance variable, groups, which is defined as follows with the given type parameters:

Map<Group, Map<Username, GroupMember>> groups

Within the same enclosing class, I have a public method, addGroup(Group group), which is supposed to put a new entry into the groups map. The Group object contains all the relevant information for the entry.

The conundrum here stems from the fact that adding a new element to groups would require that I specify a concrete object for Map<Username, GroupMember> value within addGroup(), thus internally coupling the abstraction with an implementation--clearly in violation of the dependency inversion principle.

My question is then how do I avert this problem? I hesitate delegating the responsibility of defining the object to the caller (i.e. defining the method as addGroup(Group group, Map<Username, GroupMember> groupMembers) for instance) as it chips away at abstraction, may be inconsistent with the constructor injected implementation; and in my opinion, introduces unnecessary coupling with the class implementation details. Perhaps I should reconsider my design.

---

EDIT

Here's where the problem is coming from:

public class Foo {
 private final Map<Group, Map<Username, GroupMember>> groups;
 //...
 public void addGroup(Group group) {
 if (exists(group))
 throw new IllegalOperationException("Group with same name already exists");
 //Declaring a new HashMap within the class (i.e. defining the implementation within a class member)
 groups.put(group, new HashMap<Username, GroupMember>(Map evaluation based on group object)));
 }
 //...
}

Here's what I'm hesitant of doing:

//Passing Map<Username, GroupMember> object decreases abstraction and exposes implementation details
public void addGroup(Group group, Map<Username, GroupMember> groupMembers) {
 if (exists(group))
 throw new IllegalOperationException("Group with same name already exists");
 groups.put(group, groupMembers));
}

Here's part of the definition for Group to further add context;

public class Group {
 private final String name;
 private final String description;
 private final Instant dateCreated;
 private final Collection<GroupMember> members;
 //... Constructor and getters for the fields
}

• bitsoflogic: I'm not sure why you're hesitant about allowing the caller to specify it, since it sounds like the caller will be the one using it. Perhaps a short code example of the class and its usage would clarify things.
• Garikai: @bitsoflogic I made some edits, hope they can better clarify what I mean
• bitsoflogic: This doesn't seem to be a Dependency Inversion violation because the callers don't need to pass in the Map<Username, GroupMember>. So, simply implementing new HashMap internally is fine. If it ever needs to change, the callers will never know nor be impacted by the change. The Dependency Inversion violation is really that you're using addGroup(Group) instead of addGroup(IGroup). Whether or not that violation is worth changing is up for debate - it's not a rule I advocate following 100%.
• BobDalgleish: You could also provide a factory in the caller, using a method like computeIfAbsent()
• Marco: Why is Foo in charge of associating usernames to group members? Shouldn't that be a responsibility of a Group?

Answer 1

Let me do a step back to get a look at the bigger picture.

You seem to have a requirement to map from Username to GroupMember instances inside a Group, and the Group instance seems to carry all information necessary for that mapping.

If that's correct, you should not expose a method signature that allows your caller to establish an incorrect association, so the addGroup(Group group, Map<Username, GroupMember> groupMembers) would be a bad idea, inviting for mis-use.

The fact that you create this Map<Group, Map<Username, GroupMember>> groups field implies to me that you have a method like

public GroupMember describeUser(Group group, Username user) { ... }

As this will only deal with things that are conceptually internals of Group instances, what about moving this functionality from the Foo into the Group class, maintaining a simpler Map there if access speed is really that important?

Answer 2

Dependency Inversion Principle(DIP) is ignoring dependencies from Low Level to High Level.

In your case, your addGroup(Group group) method has dependency to Group class. You can create an IGroup interface and implement it to Group class. Then convert your method like addGroup(IGroup group). Now your business doesn't depend concrete class from Low Level.

I hesitate delegating the responsibility of defining the object to the caller (i.e. defining the method as addGroup(Group group, Map groupMembers) for instance) as it chips away at abstraction, may be inconsistent with the constructor injected implementation; and in my opinion, introduces unnecessary coupling with the class implementation details.

We are creating many methods and calling within any other methods from library or framework. Those methods don't require(depend) their inner requirements as parameters.

In your case, you are able to do mapping(I assume its your business) in your addGroup(Group group) method without depending any other thing from caller. So, you don't need to create public void addGroup(Group group, Map<Username, GroupMember> groupMembers) method for DIP but you should use IUsername and IGroupMember instead of Username and GroupMember in addGroup(IGroup group).

• Pete Kirkham: What does "Those methods don't wait their inner needed as parameters." mean?
• Engineert: @PeteKirkham I tried to use more proper words. I hope it is more clear now.