0

Let me start by abstractly formulating the problem: I have two public interface types. One of them contains a method which receives at least two instances of the other interface type. The implementation of the method depends on the implementation of the passed objects.

Consider the following public API, which consists of two interfaces:

public interface Node {
}

public interface Tree {
    void connect(Node parent, Node child);
}

Now, I want to implement that API, like so:

public class NodeImpl implements Node {
    private final Wrapped wrapped;

    public NodeImpl(Wrapped wrapped) {
        this.wrapped = wrapped;
    }

    public Wrapped getWrapped() {
        return wrapped;
    }
}

public class TreeImpl implements Tree {
    @Override
    public void connect(Node parent, Node child) {
        // connect parent and child using the wrapped object
    }
}

public class Wrapped {
    // wrapped object which actually represents the node internally
}

I need to access the wrapped objects in the connect method, which is impossible, because the getWrapped method is not part of the API. It is an implementation detail.

So the question is: How can I implement the connect method without leaking implementation detail to the API?

Here is what I tried so far:

  • Put the connect method in the Node interface and call parent.connect(child). This gives me access to the wrapped object of the parent, however the wrapped object of the child is still not available.

  • Just assume the passed Node is of type NodeImpl and use a downcast. This seems wrong to me. There might be other Node implementations.

  • Don't put the wrapped object in the node, but use a map in the TreeImpl that maps Node's to Wrapped objects. This is basically the same as above. It breaks down as soon as a Node instance is passed to the connect method, which has no associated mapping.

Please note, that the Node interface might contain methods. However, this is unimportant for this question.

Also, please note that I am in control of both: The interface declaration as well as the implementation.


Another attempt to solve this is to convert the connect method to a addChild method in the Node interface and to make the Node interface generic:

public interface Node<T extends Node<T>> {
    void addChild(Node<T> child);
}

public class NodeImpl implements Node<NodeImpl> {
    private final Wrapped wrapped;

    public NodeImpl(Wrapped wrapped) {
        this.wrapped = wrapped;
    }

    public Wrapped getWrapped() {
        return wrapped;
    }

    @Override
    public void addChild(Node<NodeImpl> child) {
    }
}

public class Wrapped {
    // wrapped object which actually represents the node internally
}

public Node<NodeImpl> createNode() {
    return new NodeImpl(new Wrapped());
}

private void run() {
    Node<NodeImpl> parent = createNode();
    Node<NodeImpl> child = createNode();
    parent.addChild(child);
}

Node and createNode are part of the public API. NodeImpl and Wrapped should be hidden. run is the client code. As you can see, NodeImpl has to be visible to the client, so this is still a leaking abstraction.

Stefan Dollase
  • 4,530
  • 3
  • 27
  • 51

1 Answers1

1

if connect method needs to access the Wrapped object in each node, that means NodeImpl can be only connected to a NodeImpl, so no need to make it complex, add a method addChild or connect to Node interface, in NodeImpl implementation you can down cast the argument to NodeImpl, if there is a type mismatch you may throw a exception.

without down-casting you can use generics like, but i think the simple solution is to down cast the

interface NodeConnector<T extends Node>
{
  void  connect(T parent,T child);
}


public abstract class AbstractNode implements Node
{
  @Override
  public void connect(Node node)
  {

    NodeConnector<Node> nodeConnector = getNodeConnector();
    nodeConnector.connect(this, node);
    Node parent = this;
  }

  protected abstract NodeConnector<Node> getNodeConnector();


}

class NodeImpl extends AbstractNode
{
  @SuppressWarnings("unchecked")
  protected NodeConnector<Node> getNodeConnector()
  {
    return (NodeConnector) new NodeConnectorImpl();
  }
}



class NodeConnectorImpl implements NodeConnector<NodeImpl>
{
  @Override
  public void connect(NodeImpl parent, NodeImpl child)
  {

  }
}
hunter
  • 3,963
  • 1
  • 16
  • 19
  • I would agree with this as the best type of solution, but I am assuming OP doesn't have control over the interfaces provided. – Mena Sep 01 '16 at 10:39
  • OP said that he has tried to edit the Node interface with a new method. so i think OP is the author of the Node interface. – hunter Sep 01 '16 at 10:42
  • Ah yes: "Put the connect method in the Node interface" etc. Will delete mine and upvote yours then. – Mena Sep 01 '16 at 10:47
  • Thanks for your answer. Indeed, I am in control of the interface declaration. However, this solution still requires me to expose NodeImpl to the client: At some point, I have to create the node. This factory method then has to return a `Node`. I will add an example to the question. – Stefan Dollase Sep 01 '16 at 10:56
  • Again, since `NodeConnector` is exposed to the client and receives the concrete `Node` implementation as a type parameter, the concrete `Node` implementation has to be visible to the client. In your code, this is prevented by using an unchecked cast, which ignores the not matching type parameter. However, this is now basically a complicated downcast :-P – Stefan Dollase Sep 01 '16 at 12:31
  • i agree that there is a down cast here, but why do we need to expose the NodeConnector to API client, NodeConnetor is just a part of AbstractNode, it is sufficient only to expose the NodeFactory to client then client can create NodeImpls, but they will see them as Nodes – hunter Sep 01 '16 at 12:46
  • @hunter You are correct: `NodeConnector` can be hidden. However, this is still just a complicated downcast. I don't see the advantage over a simple downcast. – Stefan Dollase Sep 01 '16 at 13:01
  • 1
    Yes, i agree, that's why i prefer the simple downcast over this solution. – hunter Sep 01 '16 at 13:15