Simple PHP inheritance using an abstract class

Question

I have created simple class inheritance of an abstract class Pet which is basically the skeleton of all other children classes.

abstract class Pet {

    private static $relations = array(
        'Cat',
        'Dog'
    );

    public static final function getClassById($id){
        return self::$relations[$id];
    }

    protected static $foods = array(

    );

    public static function getEdibleFoods(){
        $class = get_called_class();
        return $class::$foods;
    }

    abstract public function play();
    abstract public function eat();
    abstract public function sleep();

}

Then I have the Dog and Cat classes

class Dog extends Pet {

    protected static $foods = array(
        'bones'
    );

    public function eat() { }

    public function play() { }

    public function sleep() { }

}

class Cat extends Pet {

    protected static $foods = array(
        'fish'
    );

    public function eat() { }

    public function play() { }

    public function sleep() { }

}

As you see, the only difference between them is the foods that they like. My concern about this code is the getEdibleFoods static method in the parent class.

public static function getEdibleFoods(){
    $class = get_called_class();
    return $class::$foods;
}

In order to not have to implement it in all children classes, I figured I can use get_called_class() instead of self to return the foods static variable. Notice that every child class has its own foods array and they are not all the same.

My problem is that this does not look like clean code. I'd like to ask if this is good practice and if not, how is it done the right way?

Answer 1

You're assumption that your code is messy is indeed correct. Let's examine how we can fix this:

class A {
    protected static $str = "str1";
    public static function getString() {
        return static::$str;
    }
}
class B extends A {
    protected static $str = "str2";
}
echo B::getString();

Running this will print "arr2" each time. Why?

It's the static:: keyword that does the magic. It's a simple trick called late static binding. It's basically a way to carry parent attributes down the structure of the classes.

Here's the PHP docs, which also explains self:: versus static::. It's a good read.

I'd like to ask if this is good practice and if not

It all depends, everything has a purpose and a correct time to use it. It's up to you to determine if what you're doing is the best implementation.

Would your example be correct usage, yes. However, many people are unfamiliar with late static bindings, and so it could be easy to misinterpret or skip that line if a new developer is reading your code. Perhaps there's a way you could reevaluate your code to see if you can restructure it in a way that does not depend on this functionality.

Answer 2

I think the true problem is that the method shouldn't be static at all.

---

You should think about static methods as namespaced functions. They logically belong in the same category as the class they're defined on, but they do not actually operate on objects of that class. This means that their use (at least proper use) actually comes up a lot less than you'd think it would.

Typically static methods tend to be utility functions and this is for good reason. Utility methods are basically the only thing that don't have state, and things with state shouldn't be static. As a result of this, if a method is static, it should be idempotent.

---

Another concern in usability. Having a static method exist on a class hierarchy rather than just the top level ends up in a rather odd situation. Consider code consuming your Pet hierarchy. Obviously this hierarchy exists for the sake of polymorphism, so we can assume that your code takes objects by type Pet. Guess what? This makes calling getEdibleFoods incredibly unwieldy.

function doSomethingWithPetFoods(Pet $pet) {
    // How in the world do I call getEdibleFoods?
    // get_class($pet)::getEdibleFoods? Ew.
}

---

Don't forget that static methods also equal strong coupling (conveniently, you typically don't care if your coupled to a utility method). It's a bit hard to illustrate with a non-contrived example, so you'll have to bear with me.

function calculateSalesPrice($price, $tax) {
    return Math::round($price * (1 + $tax), 2); // Round to 2 digits
}

What could be wrong with this? It's just a simple round. The problem is that the code is now coupled to a very specific round. What if the United States demands that sales prices be rounded up to the nearest penny, and Argentina dictates that sales prices be rounded down to the nearest penny.

While this is a rather contrived example and its relevence to your code is rather limited, it's worth remembering.

---

Another problem is that you're (probably) not asking "what do cats eat?" when you call it. You're asking "what does this cat eat." It's a subtle difference, but an important one. What if two dogs eat different things? Or what if a certain cat is allergic to fish? This is why you have to be incredibly sure that a static method has absolutely not relation to state.