Search code examples
c#design-patternscomposite

Understanding Composite Pattern via C#, Remove(T item) function implementation


I am working on the Composite Pattern, using the examples from C# 3.0 Design Patterns by Judith Bishop (Page 55).

The question I am stacking on concerns the function from Composite class: public IComponent Remove(T item) What is the sense of making the following IF statement

 if (_holder != null)
            {
                (_holder as Composite<T>)._items.Remove(p);
                return _holder;
            }

in

public IComponent<T> Remove(T item)
    {
        _holder = this;
        IComponent<T> p = _holder.Find(item);
        if (_holder != null)
        {
            (_holder as Composite<T>)._items.Remove(p);
            return _holder;
        }
        else
        {
            return this;
        }
    }

    //Recursively looks for an item
    //Returns its reference or else null
    public IComponent<T> Find(T item)
    {
        _holder = this;
        if(Item.Equals(item))
        {
            return this;
        }
        IComponent<T> found = null;
        foreach(IComponent<T> comp in _items)
        {
            found = comp.Find(item);
            if(found != null)
            {
                break;
            }
        }
        return found;
    }

As far as I am concerned, _holder variable is always assigned in both Remove and Find functions and therefore cannot be null. Do they mean p referece to be checked for null instead?

Interface IComponent

 public interface IComponent<T>
{
    void Add(IComponent<T> component);
    IComponent<T> Remove(T s);
    IComponent<T> Find(T s);
    string Display(int depth);
    T Item { get; set; }
}

Component class implementation:

    class Component<T> : IComponent<T>
{
    public T Item { get; set; }

    public Component(T item)
    {
        Item = item;
    }

    public void Add(IComponent<T> item)
    {
        Console.WriteLine("Cannot add to Single item!");
    }

    public IComponent<T> Remove(T item)
    {
        Console.WriteLine("Cannot remove directly the Single item");
        return this;
    }

    public IComponent<T> Find(T item)
    {
        if (Item.Equals(item))
        {
            return this;
        }
        return null;
    }

    public string Display(int depth)
    {
        return string.Format("-{0}---{1}", depth, Item);
    }
}

Composite class implementation:

public class Composite<T> : IComponent<T>
{
    public T Item { get; set; }
    private List<IComponent<T>> _items = new List<IComponent<T>>();
    private IComponent<T> _holder;

    public Composite(T item)
    {
        Item = item;
    }

    public void Add(IComponent<T> item)
    {
        _items.Add(item);
    }

    //Finds the item from a particular point in the structure
    //and returns the composite from which it was removed
    //If not found, return the point as given
    public IComponent<T> Remove(T item)
    {
        _holder = this;
        IComponent<T> p = _holder.Find(item);
        if (_holder != null)
        {
            (_holder as Composite<T>)._items.Remove(p);
            return _holder;
        }
        else
        {
            return this;
        }
    }

    //Recursively looks for an item
    //Returns its reference or else null
    public IComponent<T> Find(T item)
    {
        _holder = this;
        if(Item.Equals(item))
        {
            return this;
        }
        IComponent<T> found = null;
        foreach(IComponent<T> comp in _items)
        {
            found = comp.Find(item);
            if(found != null)
            {
                break;
            }
        }
        return found;
    }
}

Solution

  • The check is meant for p and not this because this cannot be null.

    _holder = this;
        IComponent<T> p = _holder.Find(item);
        if (_holder != null)
    

    this cannot be null so _holder cannot be null either but the find can return null and it is very logical to check the result of find before doing a remove.

    Another possibility is that

    _holder as Composite<T> is null ?
    

    so

    if (_holder as Composite<T>==null) 
    

    is also a valid check.