Common methods in parent class

Question

Imagine this simple PHP class:

<?php
class MyParent
{
 // this is our common method, we don't use this method anywhere in parent class
 // but we may or may not use this method in some child classes.
 protected function helperMethod()
 {
 // code
 }
}

How is it to have methods like top one in our classes? Is this OOP? I don't have good feeling about this, cause if we don't use these methods in our child classes they will be completely useless and they can confuse people. please tell me your idea.

Another example:

abstract class Package
{
 // We will never use this method in this class
 protected function getAConfigParam()
 {
 ConfigHolder->getParam('myconfig');
 }
 public function register()
 {
 [...]
 this->registerPackage();
 [...]
 }
 abstract public function registerPackage();
}
class PackageForCli extends Package
{
 public function registerPackage()
 {
 // We will use getAConfigParam method here.
 }
}
class PackageForWeb extends Package
{
 public function registerPackage()
 {
 // We will use getAConfigParam method here.
 }
}

In the top example we have also access to the ConfigHolder in child classes, but which one should we use? getAConfigParam method or ConfigHolder?

• Tulains Córdova: One makes classes that are "children" of a "parent" class for a reason. What's your reason? Looks like your "children" classes shouldn't have that parent after all.
• Laiv: It's totally code smell. The function is going to be better placed in a utility-like class. As soon as it's pure and stateless.
• Tulains Córdova: "we don't use this method anywhere in parent class" sometimes a class exposes a method that it itself doesn't use, but "clients" will use.
• Tulains Córdova: I thin that if you don't put a real name for "MyParent" and for "helperMethod", an useful question will be impossible. Only with context can we say if something is wrong.
• Tulains Córdova: As "MyParent" and "helperMethod" say nothing about the context, only you know what the situation is.

Answer 1

If I'm right, your doing something called template method . It's a OOP design pattern. So far so good.

In the template method of this design pattern, one or more algorithm steps can be overridden by subclasses to allow differing behaviors while ensuring that the overarching algorithm is still followed

What we miss in your case is if the configuration is required to ensure that the overarching registration process is follwed.

If it is required, then you have to inject it into the template method.

abstract class Package
{
 public function register()
 {
 [...]
 this->registerPackage(ConfigHolder->getParam('myconfig'));
 [...]
 }
 abstract public function registerPackage(ConfigParam cfgParam);
}
class PackageForCli extends Package
{
 public function registerPackage(ConfigParam cfgParam)
 {
 // We will use getAConfigParam method here.
 }
}
class PackageForWeb extends Package
{
 public function registerPackage(ConfigParam cfgParam)
 {
 // We will use getAConfigParam method here.
 }
}

Now, no common method in the super class is need it. At least not the one we are talking about.

The above solution, wont stop a developer from doing this:

 class PackageForMobile extends Package
 {
 public function registerPackage(ConfigParam cfgParam)
 {
 //ignoring ConfigParam
 ConfigHolder->getParam('myconfig');
 }
 }

Or totally ignore the ConfigParam whether you deem it necessary or not.

If you find the child classes to be ignoring the ConfigParam systematically, then remove the access to the param. YAGNI.

If you find the child classes eventually accessing to the ConfigParam, consider allowing them to get access to it via DI and remove the ConfigParam from the template. YAGNI again. Just one more consideration, inject the ConfigParam into the children instead of calling the the configHolder directly. It's easier to unit test with DI.

You didn't say if getAConfigurationParam() is required by the children's consumers. If consumers do need access to the ConfigParam, then the method is still necessary. That depends on your needs and requirements. But it should not affect the overarching registration process.

• Sina Sharifzade: If by consumers you mean users outside of the class, I have to say no, this method is only necessary for child classes this is why it is protected.
• Laiv: Then you know what to do ;-).