Is this bad Object Oriented Design? (derived class used in base class)

Question

Hello, Can someone tell me if this class structure is bad?

class abstract Parent{
  public Child Foo(){
    return new Child();
  }
}

class Child : Parent{}

I've heard that referring to a derived type from a base type is always bad and signs of bad design. Can someone tell me why this is bad or even if it is bad?

Answer 1

It looks to me like you are using the base class as a factory. I would recommend against this design because it reduces the cohesion of the base class (it's both base class and factory) and increases coupling in the base class (as it references the derived class(es)).

Create a factory class separate from the base class and these issues go away.

Answer 2

To elaborate on dkackman's answer, your factory ideally would return objects of child types, but declared as the parent type.

class Factory
{
    public Parent Foo()
    {
        return new Child();
    }

    public Parent Bar()
    {
        return new OtherChild();
    }
}

The basic idea is that your calling code shouldn't care which child class it gets back. This is one concept of the Liskov Substitution Principle.

Answer 3

It certainly smells bad, in many levels. Just ask yourself what would happen if someone extends Child ; or it another subclass of Parent (instead of Child) is made.

It's hard to imagine a case that would justify this design (perhaps it exists, you could explain what you are trying to achieve). (Are you familiar with the factory pattern?)

In any case, to get a reasonable behaviour for such design I guess one should accept and embrace the coupling, even try to enforce it, by making the Child class final/sealed (impossible to extend) and thinking both classes as a whole. But then again, that could (almost surely) better be achieved with another cleaner design.

Answer 4

I can't say anything wrong on your design. To say more concretely I must know your purpose of doing so. But whatever your purpose is, you will miss polymorphism. So, think from the client code perspective, you are exposing super class details just to get the sub-class instance. Why on earth we do that? Atleast, I don't. Remember the design principle, we always want a highly cohesive class. Here you are breaking that principle by providing factory method to create a sub-class instance.

rajan