Preferred method of accessing an external class of methods through the original class

Question

I want to move a bunch of similar methods to an external class. The class is initialized with the original class instance. From there I can access it either by property (persistent instance) or by method (new instance each time).

Which approach is favorable? In a real system I will have many of these classes holding many methods. I would like to hear your opinions and knowledge.

EXAMPLE:

I have a class, let's call it Zoo, and in my Zoo class I hold Lists of animals.

(These animals have no common properties or methods where an interface or inheritance would be applicable, this is purely to paint an example.)

public class Zoo
{
    public List<Monkey> MonkeyList;
    public List<Lion> LionList;
    public List<Turtle> TurtleList;

    public Zoo()
    {
        MonkeyList = new List<Monkey>();
        LionList = new List<Lion>();
        TurtleList = new List<Turtle>();
    }
}

Now I want to add animals to my Zoo, so I create some Add methods.

Instead of putting them directly to my Zoo class, I add them to a separate class and pass my Zoo instance, like so:

public class ZooAdd
{
    private Zoo _zoo;

    public ZooAdd(Zoo zoo) 
    {
        _zoo = zoo;
    }

    public void Monkey(Monkey monkey)
    {
        _zoo.MonkeyList.Add(monkey);
    }

    public void Lion(Lion lion)
    {
        _zoo.LionList.Add(lion);
    }

    public void Turtle(Turtle turtle)
    {
        _zoo.TurtleList.Add(turtle);
    }       
}

Now I need to add the animals, but faced with two options, unsure which is preferable:

Option 1: I create a property

public class Zoo
{
    public ZooAdd Add;

    public Zoo
    {
        Add = new ZooAdd(this);
    }
}

...

Zoo zoo = new Zoo();
Monkey monkey = new Monkey();

zoo.Add.Monkey(monkey);

Option 2: I create a method returning an instance (Factory pattern?)

public class Zoo
{
    public ZooAdd Add()
    {
        return new ZooAdd(this);
    }
}

...

Zoo zoo = new Zoo();
Monkey monkey = new Monkey();

zoo.Add().Monkey(monkey);

Both approaches satisfy the requirements and have little difference in appearance.

I believe that the second option may prove more useful for memory efficiency as the object created through the method is presumably eaten up by the GC after it's used but I have no concrete knowledge regarding that statement.

Please, lend me your wisdom.

EDIT:

You've all raised some justified concerns about my approach and I agree that it can be improved. I will be revising the structure of my classes with your suggestions and assistance in mind. Thank you all for your answers.

Answer 1

Can I ask why you are trying to do this? It is hard to give you guidance without knowing the motivation. But in both options you are creating an object and then throwing it away. Both options are...not optimal.

If your motivation is to separate the code, there are partial classes in some languages. You could also make a base class that provides all the add methods.

Some unsolicited advice though...methods should be verbs, action words. Monkey is not a verb. What action is taken when I call Monkey? If you are in a language that supports it, I'd just overload Add with the different animal types. Otherwise AddAnimalXXX would be a better method name as it describes what action the method is taking. Self documenting code FTW! Properties should not have side effects, so Add is a bad property in Option 1.

Answer 2

In your example there isnt much difference, but I generally find option 2 hard to work with.

Its becomes most obvious when the object has properties. eg

Zoo.Add().BannanasIncluded = 2

Doesnt work, so you are forced to make it immutable

Zoo.Add(bannanasIncuded:2).Monkey()

Also, how do I know not to make my own adder?

var x = new ZooAdder(myZoo)

or keep a reference to one after its been used?

var x = Zoo.Add(bannanasIncuded:2)
x.Monkey()
x.bannanasIncuded = 3 //why cant I do this??!?
x.Monkey()

Or just use the field directly?

Zoo.MonkeyList.Add(new Monkey())

Also from a performance standpoint newing and collecting millions of adder objects isnt great.

Secondly, I guess you are creating these classes to add a fluent interface rather than add functionality?

While thats fine and some people love them, its not essential to the class and it would be better to make it a seperate dll if possible.

Perhaps you could make it an extension method instead. Then I can add monkeys without any monkey business.

Answer 3

If you go with ZooAdd aproach you'll violate Open Close principle: while adding new animal classes to Zoo you would also need to add new methods to ZooAdd. I don't fully understand what you're trying to achieve, but IMO ZooAdd is not the best way to solve your problem. If you go that rote you will need to maintain additionall class and it's tests, and the only gain would be syntatic sugar in form of:

zoo.Add.Monkey(monkey)

which can be seen as violation of Law of Demeter and can make you some more trouble in the future.

While assigning responsibilites in oo languages it's good to use Information Expert as a guidance and while designing messages calls between objects it's good to remember about Law of Demeter.

Imo you should go with plane and simple:

public class Zoo
{
    public List<Monkey> Monkeys;
    public List<Lion> Lions;
    public List<Turtle> Turtles;

    public Zoo()
    {
        Monkeys = new List<Monkey>();
        Lions = new List<Lion>();
        Turtles = new List<Turtle>();
    }
}

and then you can:

Zoo zoo = new Zoo();
Monkey monkey = new Monkey();

zoo.Monkeys.Add(monkey);

Answer 4

Lots of repetition of code implies to me that there's might be an abstraction missing.

My initial thought is to have:

• An Animal base class
• A single List<Animal> in Zoo
• A single Add(Animal animal) method in Zoo
• Inherit off the Animal base class for Monkey, Lion, and any other animals you might want to add in future.

Alternatively, something I've done myself in the past is to use something like the Text Template Transformation Toolkit (T4) to generate repetitive code, and to put non-generated code in a partial class. I generally only use that when repetition is unavoidable.