patternphpMinor
Simple PHP inheritance using an abstract class
Viewed 0 times
simpleabstractphpinheritanceusingclass
Problem
I have created simple class inheritance of an abstract class
Then I have the
As you see, the only difference between them is the foods that they like. My concern about this code is the
In order to not have to implement it in all children classes, I figured I can use
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?
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 classesclass 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?
Solution
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
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.
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.
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.
Code Snippets
function doSomethingWithPetFoods(Pet $pet) {
// How in the world do I call getEdibleFoods?
// get_class($pet)::getEdibleFoods? Ew.
}function calculateSalesPrice($price, $tax) {
return Math::round($price * (1 + $tax), 2); // Round to 2 digits
}Context
StackExchange Code Review Q#60950, answer score: 7
Revisions (0)
No revisions yet.