1

we have a hierarchical structure where every node is derived from a node base class public Node(INodeFactory nodeFactory). The factory is injected so the node can create its child nodes using Get(int id)

At a later moment some nodes need a reference to another node in the structure. The information necessary to get the reference might change at runtime and is not necessarily available When the node object was constructed. Basically this method signature looks the same and is Get(int id). This time no new object should be created but an existing one should be returned.

Our first attempt was to pass a INodeLocator that would search for the node. First of all we are not sure 'locator' is a good name and if we are missing some pattern here, maybe the repository pattern (but only to look up?). Second we noticed the method signature is the same.

We were considering to switch the factory from 'creation' to 'lookup' mode after the initial tree has been created but that won't work since later on nodes need to be created as well.

For the 'locator' logic we were thinking of searching (iterating) through the nodes but maybe it is better to keep track of them in a flat dictionary. But then the problem arises that the factory can add to the dictionary but doesn't manage the lifetime. What should happen when a node gets removed.

Edit after Vladimir Stokic's answer: We're currently thinking of two solutions to keep the dictionary clean. One is to have a dictionary of weak references but then we just move the problem, there will be weak references that need to be removed (using a timer tick from time to time?) Second idea is to have the node notify the factory of deletion.

How can we design for this problem in a proper way?

  • Why not just have one factory service and one "locator" service ? Or even a lookup method instead off Get ? – Walfrat Apr 5 '17 at 12:06
  • 3
    Make the pattern fit the situation, not the other way around. Your GetId(int) can easily return an already instantiated object. You might wanna stop calling it a factory though, NodeReader perhaps. – Chris Wohlert Apr 5 '17 at 12:17
  • @Walfrat There's no real issue in having them both. It just 'feels' strange to have two things that look so identical. From a consumer point of view there is no difference, the consumer should not know or care if it is a 'new' node or a 'lookup' node. So it shouldn't really care which service to call. – Jef Patat Apr 5 '17 at 12:23
  • @ChrisWohlert That's indeed the question summarized, if we merge the factory an the locator then what are we creating? This doesn't sound like something that hasn't been done a dozen times. – Jef Patat Apr 5 '17 at 12:24
  • A factory can cache the object that are already been created, however, when you lookup, do you need to have an error if the ID is wrong ? If the ID is unique through the whole hierarchy, the factory can just store the built Node in a Dictionary<ID,Node> and perform a lookup everytime you ask for a Get, if it deson't exists it creates a new one. – Walfrat Apr 5 '17 at 12:41
4

I don't believe there is a single pattern that would fit the bill here. However, you might use a combination of patterns. What you really need is not a node creator, but more of a node provider.

The node can be provided by creating a new one, or finding an existing one and providing a reference to it. Therefore, you should first abstract that functionality in one class, let's call it NodeProvider. This class would have instances of NodeLocator and NodeFactory. The Get(int id) method of NodeProvider class would first invoke the Get(int id) method of the NodeLocator member. If it does not locate the existing node, then Get(int id) method of the NodeFactory member. Either way, a node is provided. The design pattern that this is most similar to, in my opinion, is Facade Pattern.

Pseudo code (without any checks) is given below:

class NodeProvider
{
    private NodeLocator locator = new NodeLocator();
    private NodeFactory factory = new NodeFactory();

    public Node Get(int id)
    {
        Node retVal = null;
        retVal = locator.Get(id);
        if (retVal == null)
        {
            retVal = factory.Get(id);
        }
        return retVal;
    }
}
  • I very much like the 'provider' name. That covers the load very well. I'm not flagging your answer as answer yet because I'm not sure how the dictionary/delete problem should be handled. I will adapt the question with our current thoughts. (Edit: would vote your answer but not enough reputation yet) – Jef Patat Apr 5 '17 at 14:18
  • @JefPatat the delete problem is solved the same way. Add a Delete method to the NodeProvider, and use that to delete Nodes. If the NodeProvider is no longer available, keep a reference to the "owning provider" in Node. – Frank Hileman Apr 5 '17 at 19:29
  • Is it good design to have the provider add to the dictionary but rely on the nodes to (indirectly) delete from the dictionary? – Jef Patat Apr 5 '17 at 19:46
  • That depends on what the trigger for deleting from the dictionary would be. Can you provide more details on that? – Vladimir Stokic Apr 6 '17 at 8:06
0

The factory pattern seems inapproprate to me altogether in your scenario. Child nodes can be spawned locally, by their parent nodes (making child production a feature of node, you cannot get more natural than that). Node lookup is something entirely different/separate. Joining the two in a single class does not make sense and would violate SRP.

  • creating childs is not the single responsibility of a node, on the contrary, its responsibility is to represent a node in the hierarchy somehow for example having a name (in our case a node has many more properties and contains logic that allows managing childs, copy/paste, move, ...). The two together makes sense if you see the responsibility as 'providing a node'. – Jef Patat Apr 5 '17 at 19:45
  • 1
    So, your node class does copy, paste, move and what more to and with nodes, but for some reason you want to externalize the trivial acts of creating and retrieving. It sound like you already have a node manager in your node class so why not have it create and find nodes too? – Martin Maat Apr 5 '17 at 21:02
  • The question is a simplified example. In reality this is a VM structure, node modification is triggered from the outside, both from lower and upper layers. One very good reason would be that this means the base class needs to be able to create new instances of inherited classes. – Jef Patat Apr 6 '17 at 6:21

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.