tags:

views:

66

answers:

3

I have a scenario where I have several classes that extend from a common ancestor because they all share common properties and methods. Half of these classes (Group A) contain a property that is not shared by the other half (Group B), so each class in Group A explicitly declares this property rather than the parent class.

e.g.

class parent
{

}

class child1 <and child2, and child3> extends parent
{
    protected $specialProperty;
}

class child4 <and child5, and child5> extends parent
{
    // no "$specialProperty"
}

I have some behavior I need to implement that acts on $specialProperty. Naturally, I'd like to put this in a parent class and do it only once, but $specialProperty doesn't exist in all of the child classes.

One option is to create an intermediate, abstract class that declares $specialProperty and implements the required behavior, but I'm reluctant to do that for a few reasons:

  1. This scenario is going to be coming up more soon. What do I do then? Create 5 or 6 different middlemen abstract classes for each of these functionalities?
  2. The class hierarchy is already about 5 levels deep above parent. Should I really be creating more and more layers to make up for a less-than-ideal class hierarchy that was conceived hastefully with limited requirements?
  3. At least as far as PHP is concerned, wouldn't so many layers of inheritance eventually lead to performance issues?

Another option is to do the following:

class parent
{
    public function functionThatTheAppWillCallAutomatically()
    {
       if ( property_exists($this, 'specialProperty') )
       {
           // do stuff with specialProperty
       }
    }
}

The "stuff" would get executed by child classes that have $specialProperty and skipped by those that don't. I hate this solution though because it just seems wrong and sloppy to me. In my opinion, the parent shouldn't be deciding what to do based on the properties of the child (heck, it probably shouldn't even know that the child exists - isn't that the reason for the child class in the first place?)

In short, I'm not sure which of these two options is the least bad, or if there's a better possibility. I'm anxiously awaiting suggestions.

Thanks!


EDIT:

I ended up doing the following to implement this specialized functionality.

class archivalDecorator extends decoratorBase /* decoratorBase just has constructor and the object property */
{
    public function archive()
    {
        if ( !$this->object->archive() )
        {
            return false;
        }

        if ( property_exists($this->object, 'specialProperty') )
        {
            // do extra stuff here that involves "specialProperty"
        }

        return true;
    }
}

This way, all of my objects perform the same archival workflow, but the classes that require special behavior can still perform it without me needing to implement dozens of subclasses in the hierarchy for these special cases.

Though I'm still using property_exists(...) to determine if the special behavior is needed or not, I think this is OK now because I'm not doing it from within the parent class. $specialProperty is a public property, so there's no reason external classes shouldn't know about it, but something just felt wrong about the parent class checking for that property in a child class using $this.

I hope I haven't misapplied the concept.

+1  A: 

It's really hard to say without know what your classes are doing, but your issues (deep class hierarchies with multiple subtrees, optional cross-tree functionality, etc) are indicative of the need for some decorator pattern.

Peter Bailey
If I use a decorator, I would still need the code applying the decorator to check whether or not the object needs to be decorated. That is, if it has "$specialProperty", then I need to apply the decorator and call the specialized method, else I proceed as normal.So, I technically still need to have some logic to look into the child class, but the difference here would be that I'm doing it externally rather than from the parent.I suppose this is what makes it architecturally sound. Is this correct?
Mike
That's not how the decorator pattern works. You'd rework your entire class trees if you went this route.
Peter Bailey
@Peter Bailey: Can you be more specific as to where I've gotten it wrong? Should I instead replace the call to "functionThatTheAppWillCallAutomatically()" with a call to the decorator (vs only when "$specialProperty" is present) and have the decorator decide how to perform the action based on whether or not "$specialProperty" is declared in the object? Or am I completely off here?
Mike
How familiar are you with the decorator pattern? It's not something you can apply "here and there". It's a way to achieve multiple-inheritance-like behavior in languages that support only single inheritance. Again, I'm not totally sure if decorator is right for you, but if it is, you're going to have to do some serious re-plumbing of your class trees. If you haven't already looked at some basics of how to implement decorator, I suggest you do that now.
Peter Bailey
I've read several of the links in the search results link you provided and I know the basics of how to implement it. Suppose all of my classes need an archive() method. They all do it the same way, but half need to do something with $specialProperty. It would seem the ArchivalDecorator would do "$this->object->archive()" followed by "if (!property_exists($this->object, 'specialProperty')) { //do extra stuff }. This seems like the right place to apply Decorator, but you're saying it's not. I'm not allowed to just go in and redo the whole tree, so I just want to avoid adding to the mess. :)
Mike
Perhaps I misunderstood you. If you've done the reading and can picture the class relationships in your head, then I say sally forth.
Peter Bailey
Great. Thanks a lot!
Mike
+1  A: 

I'm not too familiar with PHP and there aren't any specifics of the class hierarchy, but I want to comment on your concerns about the depth of the class hierarchy.

If whatever you are modeling is heterogeneous enough in terms of its properties that you end up with a deep class hierarchy, then there is no sense placing an artificial cap and saying "five levels are ok but not six", or "x many middlemen are too much". Unless you can abstract certain differences and not represent them at all, thus placing multiple instances into the same "equivalence classes", you have to model the split somewhere, and this may be as good as any. If you don't do it in the type hierarchy, you might end up with type checks or extra properties.

If you do find yourself with too complex a hierarchy, because the presence of certain properties overlap, you may want to look at multiple inheritance (or multiple interface implementation) like the decorator pattern, if that is supported conveniently in PHP.

Uri
A: 

Ok, so you have a special property that only exists in some class and you have a method that has to act on this special property but you don't know where to put it?
What about in the class you define the special property?

class child1 <and child2, and child3> extends parent
{
    protected $specialProperty;

    public function doSomethingWithSpecialProperty() {

   }
}

Another option would be to override the parent's method:

class child1 <and child2, and child3> extends parent
{
    protected $specialProperty;

    public function functionThatTheAppWillCallAutomatically() {
        parent::functionThatTheAppWillCallAutomatically();
        // do other stuff
    }
}

You can also create an empty method that acts somehow as placeholder and implement it in the child classes where necessary:

class parent
{
    public function functionThatTheAppWillCallAutomatically()
    {
       $this->extraStuff();
    }
    protected function extraStuff(){};
}


 class child1 <and child2, and child3> extends parent
 {
    protected $specialProperty;

    protected function extraStuff() {
        echo $specialProperty,
    }
 }

Otherwise you might have to think about your design again.

Felix Kling
The property is not declared in a single class. I have 7 classes declaring this property and 7 that do not, but they all inherit from the same parent. I don't want to copy and paste the same method into 7 classes.
Mike
@Mike: Then the most OOP-like way would indeed to create some kind of intermediate class. Or, as I said, rethink your design.
Felix Kling