Father class with the same responsibility of the daughter class

Question

I got my class Item

public class Item
{
   public string Nome {get;set;}
   public decimal Valor {get;set;}

   public ICollection<SubItem> SubItens {get;set;}
}

public class SubItem 
{
   public string Nome {get;set;}
   public decimal Valor {get;set;}
}

I have these two classes

What happens is, when I have SubItens, the value of the class Item should be annulled, but when I don’t have subitens the value of the class Item is the valid value.

Well, it is a bit confusing, but in my business rule it is necessary an implementation based on this.

My question is, is it right to have it? What would be the best way for me to make it easier ? Using Inheritance ?

Answer 1

I don’t know what the purpose of class but, just from your code, I think this is enough:

public class Item
{
   public string Nome {get;set;}
   public decimal Valor {get;set;}

   public ICollection<Item> SubItens {get;set;}
}

No need for a class Subitem because a Subitem is no more than a item.
Conclusion you already drew when you admitted that Item and Subitem have the same responsibility

Answer 2

The way the class Item is structured, even applying the @ramaral solution, it will be very difficult for the customer to use the item tree. Customer will always have to check if an item is a node in the tree (if(item.SubItems != null)) or if the item is a leaf from the tree.

For example, to print the entire tree, the client would have to define a recursive method, and at each iteration determine the type of item.

public void PrintNode(Item item)
{
    if(item.SubItems == null)               //é folha?
        Console.WriteLine(item.Name);
    else                                    //é nó?
        foreach(var subItem in item.SubItems)
            PrintNode(subItem);
}

Item root = //criar a arvore
PrintNode(root);

The ideal would be to restructure the class Item in order to expose only handling methods, and apply the standard Composite, in order to treat all items, either nodes or leaves, uniformly.

public interface IItem
{
    void Do(Action<Item> action);
}

public class ItemComposite : IItem
{
    private readonly IEnumerable<IItem> _subItems;

    public ItemComposite(IEnumerable<IItem> subItems)
    {
        _subItems = subItems;
    }

    public void Do(Action<Item> action)
    {
        foreach(var subItem in _subItems)
            subItem.Do(action);
    }
}

public class Item : IItem
{
    public string Nome {get; private set;}
    public decimal Valor {get; private set;}

    public Item(string nome, decimal valor)
    {
        Nome = nome;
        Valor = valor;
    }

    public void Do(Action<Item> action)
    {
        action(this);
    }
}

Client code to print tree:

IItem root = //construir árvore 
root.Do(item => Console.WriteLine(item.Nome));

Another possible implementation with support for IEnumerable<T>

public interface IItem : IEnumerable<Item>
{
}

public class ItemComposite : IItem
{
    private readonly IEnumerable<IItem> _subItems;

    public ItemComposite(IEnumerable<IItem> subItems)
    {
        _subItems = subItems;
    }

    public IEnumerator<Item> GetEnumerator()
    {
        return _subItems.SelectMany(subItem => subItem).GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

public class Item : IItem
{
    public string Nome { get; private set; }
    public decimal Valor { get; private set; }

    public Item(string nome, decimal valor)
    {
        Nome = nome;
        Valor = valor;
    }

    public IEnumerator<Item> GetEnumerator()
    {
        yield return this;
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

Code of the client

IItem root = //...

foreach(Item item in root)
    Console.WriteLine(item.Nome);

Answer 3

you can put a little logic in get of Valor of Item.

In the example below, if the object Item, own some SubItem, the property Valor of Item will return the sum of properties Valor of their SubItem

public class Item
{
    private decimal valor;
    public string Nome {get;set;}
    public decimal Valor 
    {
        get
        {
            if (this.SubItens != null && SubItens.Any())
                return this.SubItens.Select(subItem => subItem.Valor).Sum();
            return this.valor;
        }
        set
        {
            this.valor = value;
        }
    }

    public ICollection<SubItem> SubItens {get;set;}
}

public class SubItem 
{
    public string Nome {get;set;}
    public decimal Valor {get;set;}
}

Answer 4

You can simplify for one class only:

public class Item
{
    [Key]
    public int ItemId { get; set; }
    public int? ItemPaiId { get; set; }

    public string Nome {get;set;}
    public decimal Valor {get;set;}

    public virtual Item ItemPai { get; set; }
    public virtual ICollection<Item> SubItens {get;set;}
}

I think it solves the whole problem.