2

I'm regularly facing following pattern:

public abstract class BaseItem
{
    BaseItem[] children;

    // ...

    public void DoSomethingWithStuff()
    {
        StuffCollection collection = new StuffCollection();
        foreach(child c : children) c.AddRequiredStuff(collection);
        // do something with the collection ...
    }

    public abstract void AddRequiredStuff(StuffCollection collection);
}

public class ConcreteItem : BaseItem
{
    // ...

    public override void AddRequiredStuff(StuffCollection collection)
    {
        Stuff stuff;
        // ...
        collection.Add(stuff);
    }
}

Where I would prefer something like this, for better information hiding:

public abstract class BaseItem
{
    BaseItem[] children;

    // ...

    public void DoSomethingWithStuff()
    {
        StuffCollection collection = new StuffCollection();
        foreach(child c : children) collection.AddRange(c.RequiredStuff());
        // do something with the collection ...
    }

    public abstract StuffCollection RequiredStuff();
}

public class ConcreteItem : BaseItem
{
    // ...

    public override StuffCollection RequiredStuff()
    {
        StuffCollection stuffCollection;

        Stuff stuff;
        // ...
        stuffCollection.Add(stuff);

        return stuffCollection;
    }
}

What are pros and cons of each solution?

For me, giving the implementation access to parent's information is some how disconcerting. On the other hand, initializing a new list, just to collect the items is a useless overhead ...

What is the better design? How would it change, if DoSomethingWithStuff wouldn't be part of BaseItem but a third class?

PS: there might be missing semicolons, or typos; sorry for that! The above code is not meant to be executed, but just for illustration.

  • 1
    Did you profile the code to know that the overhead is a problem? – Euphoric May 27 '14 at 17:46
  • Not yet. I assume it's not a real issue - at least if cardinality of children is reasonable small. But for me it's about best practice / pros and cons ... the size of children might, however, be a sensible basis of decision-making. – Markus May 27 '14 at 20:08
  • Why are you using StuffCollection instead of some generic collection, like List<Stuff>? – svick May 28 '14 at 19:41
  • @svick: no special reason. I'm working on a SharePoint project and nearly everything is encapsulated in a special collection. – Markus May 28 '14 at 20:11
1

initializing a new list, just to collect the items is a useless overhead

instantiate 2 class-level collections, once. The first is for the cumulative AddStuff() and the other is to pass to the child - and is reused.

public abstract class BaseItem
{
    BaseItem[] children;
    StuffCollection masterCollection;
    StuffCollection transferCollection;

    // ...

    public void DoSomethingWithStuff()
    {
        masterCollection = masterCollection?? new StuffCollection();
        transferCollection = transferCollection?? new StuffCollection();

        transferCollection.Clear();

        foreach(child c : children) {
            c.RequiredStuff(transferCollection);
            masterCollection.AddRange(transferCollection);
            transferCollection.Clear();
        }

        // do something with the collection ...
    }

    public abstract void RequiredStuff(StuffCollection aTransferCollection);
}

public class ConcreteItem : BaseItem
{
    // ...

    public override void RequiredStuff(StuffCollection collector )
    {
        Stuff stuff;
        // ...
        collector.Add(stuff);
    }
}
  • Very nice idea, haven't thought of this! – Markus May 28 '14 at 6:05

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.