2
\$\begingroup\$

For a while I was thinking of a way I could have multiple objects of different types that related to each other be included in a tree.

For example, say I have CEO, Manager, Representative which all implement interface IEmployee.

I have a tree:

class ObjectNode<T> : IEnumerable<ObjectNode<T>>
{
    private object _obj;
    private Type _type;
    private readonly ICollection<ObjectNode<T>> _children = new LinkedList<ObjectNode<T>>();

    public ObjectNode<T> Parent { get; set; }

    public ObjectNode(T data)
    {
        _obj = data;
        _type = data.GetType();
    }

    public ICollection<ObjectNode<T>> Children
    {
        get { return _children; }
    }

    public ObjectNode<T> AddChild(T data)
    {
        ObjectNode<T> childNode = new ObjectNode<T>(data) { Parent = this };
        _children.Add(childNode);
        return childNode;
    }

    public void AddChildren(params T[] children)
    {
        int len = children.Length;
        for (int i = 0; i < len; ++i)
        {
            AddChild(children[i]);
        }
    }

    // The generics are necessary here in order to properly typecast object.
    // CS0693: Type parameter "T" has the same name as the type paramater from outer type "File.ObjectNode<T>"
    #pragma warning disable 0693
    public T Object<T>()
    {
        return (T)_obj;
    }
    #pragma warning restore 0693

    public new Type GetType()
    {
        return _type;
    }

    // IEnumerable implementation.
}
  • I can't do var employee = new ObjectNode<IEmployee>(new CEO()); and then use employee.CEOMethod(); because employee is of type IEmployee.
  • I can't make employee ObjectNode<CEO> because then AddChild(T) will be expecting type CEO. And that limits my data type to only one.
  • It also will not properly take my methods, since it'll be
    expecting methods under File.ObjectNode<File.CEO>.

And say I wanted to add a child of CEO, the subordinates, which are of type Manager. How did I solve this problem? I added a "layer of abstraction."

If I wanted to access a CEO method, I could just do: employee.Object<CEO>().CEOMethod(); and it would do as I need how I need.

If I wanted to add children to CEO, I could use: var manager = employee.AddChild(new Manager { Field = 1; }) and do this X amount of times, loop through employee.Children and type check using GetType() like so:

foreach (var child in employee.Children)
        {
            if (child.GetType() == typeof(Manager))
            {
                child.Object<Manager>().ManagerMethod();
            }
            if (child.GetType() == typeof(Representative))
            {
                child.Object<Representative>().RepresentativeMethod();
            }
        }

Or simply manager.Object<Manager>.DoSomething();.

My design also allows me to have multitudes of types of IEmployee under CEO, not just Manager. CEO can have children such as AccountManager, TeamManager, etc., and the same would apply to AccountManager and TeamManager's children.

Am I doing something horribly wrong? Is this design bad? Is there a better approach that's just as flexible and allows me to traverse it like a tree? I'm trying to achieve a more efficient "data group" design implementation for entities that subordinate other entities. I feel this is almost "hacky."

NOTE: The code here is the real implementation, but the use of "Manager", "Employee", and "Representative" were examples of how it can be used.

\$\endgroup\$

closed as off-topic by t3chb0t, Graipher, Vogel612, alecxe, Toby Speight Jul 18 '17 at 9:55

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

2
\$\begingroup\$

Remarks

  • What's the point of using an interface and a generic collection if you're only going to call type-specific methods? Instead of having a ManagerMethod, RepresentativeMethod, and so on, add a single EmployeeMethod to that interface and let each class provide their own implementation. Doing type checks all over the place doesn't scale very well, and makes it difficult to add more types.
  • Why does ObjectNode<T> implement IEnumerable<ObjectNode<T>>? It already exposes its children via the Children property.
  • ICollection<T> provides modification methods, so other code can add nodes without going through your AddChild/AddChildren methods, so you can't guarantee that all parents are set properly. You'll want to use IReadOnlyCollection<T> or IReadOnlyList<T> here.
  • Related: the Parent property's setter is public, but it doesn't do anything to inform the previous and new parents about the change. Other code could easily mess up a tree.
  • Do you have a particular reason for using a linked list instead of a 'normal' list?
  • That object _obj field should be of type T.
  • There is no need to explicitly store the type of the given data. Just call its GetType() method when you need to (if at all).
  • obj.GetType() == typeof(Class) can also be written as obj is Class (with the added benefit that it won't throw an exception when obj is null).
  • Why does ObjectNode<T> override the GetType() method to return the type of its content instead of its own type? That sort of surprising behavior makes code harder to understand and maintain, and could cause subtle bugs.
  • That AddChildren method could use a foreach loop instead of a for loop.
  • Don't ignore warning 0693, use meaningful type parameter names instead. T is used a lot when there's only one type parameter, but when there are multiple it's often used as a prefix instead (for example: IDictionary<TKey, TValue>, Func<T1, T2, TResult>).

Alternative approach

A generic tree class doesn't seem to suit this particular example. You end up with less than ideal names (Subordinates and Superior make a lot more sense here than Children and Parent). It's also cumbersome to use, with lots of type checking and casting required.

I don't know what specific requirements you're working with (if any), but I'd probably go for something like this:

interface IEmployee
{
    // Assuming each employee only has a single boss:
    IEmployee Superior { get; }

    // Employees without subordinates just return an empty collection:
    IEnumerable<IEmployee> Subordinates { get; }

    // Anything else that's common to employees goes here
}

together with an (abstract) base class that implements superior/subordinate storage. Just keep things clean and simple, there's no need for wrapping things in a separate tree node class. Note that this interface is essentially a tree node, and can easily be traversed with a recursive method.

Also, rather than doing lots of type checks and calling type-specific methods, I'd make as much use of that interface as possible. What's the point of an interface if you're not going to treat those objects similarly anyway?

Finally, it looks like this was just an example, and that you're actually looking for a general solution to a broader problem. I get the impression that you're either trying to generalize too soon (a rule of thumb I once heard recommended to generalize only after seeing a similar problem trice) or that you're trying to over-engineer this. Is there a concrete problem that you want to solve?

\$\endgroup\$
  • \$\begingroup\$ The purpose of the tree was essentially to build a system for handling logic of "groups of entities" in a personal game development practice project. For example: Spawning groups with leaders, having a party system, or making preset groupings. I couldn't think of a better structure for containing this kind of data without a tree. So essentially I could copy over groups of entities and retain their information easily as well as handle the logic as one since the data would be rather tightly coupled. Of course it was a more general question, I wanted more of an insight on the general design. \$\endgroup\$ – Tristan Gibson Jul 18 '17 at 0:57
  • \$\begingroup\$ This has provided me with a lot of insight, I'll have to redo a lot of my code, try different approaches and reproduce possible problems to really get a good idea on what will fit best. I should also say that I've perhaps been mistaken in using interfaces only as a contract rather than the clearly more expansive uses it has to offer. \$\endgroup\$ – Tristan Gibson Jul 18 '17 at 1:00
  • \$\begingroup\$ Groups and leaders probably have a specific way of interacting with each other, which probably has nothing to do with how preset groupings work, so generalizing wouldn't make much sense. But if for some reason they must be treated as general nodes by some part of your code, then letting them implement a general interface for that purpose can be useful. Each could implement that interface using methods it already has (Employee could implement INode.Children by returning its Subordinates list, for example). \$\endgroup\$ – Pieter Witvoet Jul 18 '17 at 9:18

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