4
\$\begingroup\$

Here's the situation: I've got several distinct objects, each with their own responsability and (therefore) each their own dependencies. As this is code that will be implemented in 2 existing applications, I've created my own namespaces, interfaces, abstract classes and so on... the works, basically.

IMO, the objects' inheritance chains are pretty tidy, and provide a solid base structure. There is but one thing that's really "pissing me off", though. The fact that abstract methods don't allow for covariance at all. I'm using 2 abstract classes to force certain methods to occur in the child classes (like an init method, which is called from the abstract class).
Now as I said, each class has its own responsability, and I want to be able to specifically hint for a specific dependency at the childs level, and still enforce that method using the abstract class's restrictions. Especially since some of these methods shouldn't be public (thus ruling out interfaces, at any rate).

Here's a basic example:

abstract class Base
{
    protected $foo = null;
    final public function __construct(Granny $init = null)
    {
        return $this->init($init);
    }
    abstract protected function init(Granny $dependency = null);
}

So I've made the constructor final, and Type-hinted Granny which, as the name suggest, is the base class for the Base instance's dependencies.
Suppose Granny has a child: Dad, and the classes look like this:

class Granny
{
    protected $name = null;
    protected $age = null;
    public function __construct(array $vals = null)
    {
        foreach($vals as $name => $val)
        {
            $name = 'set'.ucfirst($name);
            if (method_exists($this, $name))
            {
                $this->{$name}($val);
            }
        }
        return $this;
    }
    //basic gettter && setters
    public function getAge()
    {
        return $this->age;
    }
    public function setAge($age = null)
    {
        $this->age = $age === null ? null : (int) $age;
        return $this;
    }
}

class Dad extends Granny
{
    //Dad's secret
    private $likesMom = null;
    public function getLikesMom()
    {
        return $this->likesMom;
    }
    public setLikesMom($bool = null)
    {
        $this->likesMom = $bool === null ? null : !!$bool;
        return $this;
    }
}

Now I've yet to see anyone play baseball with their grand mother, so The Ball class, which extends Base, depends on an instance of Dad, which I'll pass to the constructor (Base hints Granny, so it'll accept Dad, too). That dependency will be passed on to the init method, but to be sure the Base::Ball instance receives the correct dependency, I'd like to declare it like this:

class Ball extends Base
{
    protected function init(Dad $dependency = null)
    {
        $this->foo = $dependency;
        return $this;
    }
}

Which is where it all comes tumbling down with a fatal error, because the signatures don't match. Which, IMO, is inconsistent behaviour (since the final public function __construct(Granny $foo = null) doesn't complain, so I can't but pass an instance of Granny)
Anyway I know the code above is said to "violate the contract", and I still maintain it's utter bull, but I've ended up doing this:

//in Dad:
protected function init(Granny $dad = null)
{
    return $this->setDad($dad);
}
//this was already defined => for DI
public function setDad(Dad $dependency = null)
{
     $this->foo = $dependency;
     return $this;
}

Just because it's more in-tune with how my code works than the, presumably, more performant:

if ($dad !== null && !$dad instanceof Dad)
{
    throw new InvalidArgumentException('You\'ll have to ask Dad to play catch');
}
$this->foo = $dad;
return $this;

But what I really want to know is: are there any other options I'm not seeing here? I'd love it if there were a sollution that allowed for covariancy, but after some googling, it's not looking good. I've even come to understand that covariant type-hints are deemed unimportant and won't be supported in the (near) future in PHP. They seem to find it more important to add return-type-hints to the method signatures, though...

\$\endgroup\$
6
  • \$\begingroup\$ Please check whats-wrong-with-overridable-method-calls-in-constructors and verify how PHP handles this issues. \$\endgroup\$ Jul 24, 2013 at 12:55
  • \$\begingroup\$ @mnhg: I'm fully aware of the pro's and cons, believe you me. I've written most of the other objects differently (with their own custom constructors), but this is a special case - honestly. I've checked how PHP handles this and what I'm trying to do is not possible ATM, I just wanted to know if there's a more elegant approach \$\endgroup\$ Jul 24, 2013 at 12:58
  • \$\begingroup\$ Maybe I'm missing something - is there a reason you have the abstract base class hinting Granny instead of a higher level object/interface that would be common to all dependencies? Why would Ball derive from a class that's dependent on Granny? It seems to me you haven't abstracted it enough. \$\endgroup\$ Jul 24, 2013 at 15:38
  • 2
    \$\begingroup\$ I think you have two options - use the instanceof to verify, but I dislike that personally. Or don't make the base class abstract, and instead throw an exception if init is called from Base. I personally wouldn't worry about someone instantiating Base. pastebin.com/6zmUruZm \$\endgroup\$ Jul 25, 2013 at 12:51
  • 1
    \$\begingroup\$ Rolled back Rev 2 → 1. Please write a comment or answer instead of spoiling the question. \$\endgroup\$ Apr 24, 2015 at 2:46

3 Answers 3

7
\$\begingroup\$

Ok, apparantly you are a professional programmer so let me look at your code first:

the __construct in the class Granny:

public function __construct(array $vals = null)
{
    foreach($vals as $name => $val)
    {
        $name = 'set'.ucfirst($name);
        if (method_exists($this, $name))
        {
            $this->{$name}($val);
        }
    }
    return $this;
}

First of, if another programmer needs to use your class he has NO IDEA how to use it. Yes, i need to add in an array of $vals. But this is just bad. I hope I don't have to explain why.. (code maintainability, readibility, ...)

then, return $this; Seriously? what else would a __construct return? void?

then: setAge($age=null). Why would I call setAge() withou passing in an age? And what is the meaning of 'null' not set?

Why are you using init() instead of simply using __construct()? Because know you are simply calling init() in the __construct() of the parent class. Whereas you could and should simply do it this way:

<?php

class Base {
    public function __construct(Granny $granny) {
        //do stuff
    }
}

Class Ball extends Base {
    public function __construct(Dad $dad) {
        //do stuff
        parent::__construct($dad);//let the parent do his magic
    }
}

Now Ball needs a spcial kind of Granny whereas Base only needs a Granny. If Dad is a Granny nothing is wrong, if Dad isn't a Granny the code will not work because $dad is not a Granny. Problem fixed.

But I bet there is some bewildered reason you didnt think of this easy 101 programming basics...

Maybe now I will have offended you, but be honest, you are doing some really weird things that will not help the guy that comes after you, or joins your project or wants to use your code. Or maybe even yourself in 4 month when you come back to the Granny class. You then have no idea what the Granny class needs and what is optional. If it is optional leave it out of the construct. Makes testing a lot easier...

So apologies If I offended you, but saying you are a professional because you work 4 years doesnt say much. I know professionals that have been coding for 4 years and still return $this in a __construct()

OLD POST:

Wow, back to the drawing board! I don't think you understand inheritance at all.

Then fun thing about OO code is that Classes can allready tell you a lot by just looking at ho wthey are declared. For isntance:

class Dog extends Mamal {}

Tells me that Dog is Mamal. And that Dog is more specific then Mamal. consider the folowing corerct phrase:

A dog can walk, but not all Mamals can (Dolphins are mamals too)

Now, I have a German shepperd class:

class GermanShepperd extends Dog {}

Again this is logical and correct.

Now lets look at your code:

class Granny

By looking at it I would say the Class Granny is some kind of stand-alone or base class. Not really specific but very abstract since it doesn't extend something.

class Dad extends Granny

Aha, Dad is a special case of Granny. Lets look into the class what it can do extra: Ah it can like a Mom. So, a Dad is a Granny that can like a Mom.

Now, you are talking about 'abstract class restrictions'. If the abstract class is giving you restrictions then maybe the abstract class isn't abstract enough. If my Mamal class needs legs to walk. Then my Dolphin with have some big troubles.

So, back to the drawing board it is. Coding isnt about tapping the keyboard and writing code. Coding is an art, and it requires a lot of think work before you code. Draw it on a piece of paper. A Granny is simply a Women that has grandchildren. Not some kind of exotic object that if it can like a Mom it suddenly becomes a Dad.

Lookup Solid It's a good place to start. Try and stick to the rules. And also, once in a while take a big step back and think 'is the thing I am doing now actually solving my problem?'. Keep it simple. If the problem is not easily solved, chop it into more problems that you can solve. Then past them togeter.

\$\endgroup\$
13
  • \$\begingroup\$ Now I must say: it looks like you've been lead astray by the example snippets I gave here. I'm using an abstract class to ensure the children have certain methods declared, like an interface, but interfaces require public methods (by their very nature). I assure you the abstract classes don't restrict the children too much, I'm looking for a more elegant way to implement the abstract method with added restrictions at the child level. I know all about SOLID, in fact convariant arguments adhere to the Liksov Substitution Principle (soLid) \$\endgroup\$ Jul 24, 2013 at 13:23
  • \$\begingroup\$ In short: the Base class hints Granny, but whilst one of Base's children might need Dad, yet another might need Mom. All I want is abstract protected function foo(Parent $instance = null); and in the child define it as protected function foo(ChildOfParent $instance = null) to ensure the correct dependency was passed. BTW: I'm a professional developer, and have spent the last 4 years writing nothing but OO code, saying I don't understand inheritance is a bit of an insult, to be honest \$\endgroup\$ Jul 24, 2013 at 13:27
  • 1
    \$\begingroup\$ You are creating problems before you even started fixing things. Just take a step back and look at the problem you are trying to solve. Can this be solved in one function? yes -> code the function. no? devide it into smaller problems. Do this recursively and you have a good program. Then apply ALL the SOLID rules to your code \$\endgroup\$
    – Pinoniq
    Jul 24, 2013 at 13:33
  • 1
    \$\begingroup\$ If Base hints Granny it means it needs Granny. If then a child needs DAD instead of Granny you simply override the method and ask for a Dad \$\endgroup\$
    – Pinoniq
    Jul 24, 2013 at 13:35
  • \$\begingroup\$ You do know that Base is abstract, right? It can't be instantiated, so I hint at the base model for all dependencies. While Ball will needs Dad, another child of Base might need Mom. init is an abstract method, BTW: by definition, the child class has to overwrite it. \$\endgroup\$ Jul 24, 2013 at 13:42
2
\$\begingroup\$

I think @Pinoniq highlight's shoud be warmly welcomed , but i think i know why you want it that way, try:

abstract class Base
{
    protected $foo = null;

    protected $_init = null;
    final public function __construct(Granny $init = null)
    {
        $this->_init =  $init;
        //The only single possible call of init
        return $this->init();
    }
    abstract protected function init();
}

class Ball extends Base {
    protected function init() {
        //so good as 'typeHint'
        if ( !($this->_init instanceof Dad) ) {
            throw new \Exception();
        }
        return $this;
    }
}

With static

abstract class Base {
    protected $foo = null;

    protected $_init = null;

    public static final function  getOne(Granny $init = null) {
        return new static($init);

    }
    protected function __construct(Granny $init = null) {}

}

class NoBall extends Base {

    protected function __construct(Granny $init = null) {
    }

}

class Ball extends Base {

    protected function __construct(Dad $init = null) {
    }

}

class Granny {}
class Dad extends Granny {}


$G = new Granny();
$D = new Dad();

$Ag = NoBall::getOne($G);
echo get_class($Ag);
$Ad = NoBall::getOne($D);
echo get_class($Ad);

$Bd = Ball::getOne($D);
echo get_class($Bd);
$Bg = Ball::getOne($G);
echo get_class($Bg);

Output

NoBallNoBallBall
Catchable fatal error: Argument 1 passed to Ball::__construct() must be an instance of Dad, instance of Granny given
\$\endgroup\$
10
  • \$\begingroup\$ Welcome cske. I hope you enjoy CodeReview. \$\endgroup\$
    – Legato
    Apr 24, 2015 at 12:29
  • \$\begingroup\$ That's no different to what I have now: abstract protected function init(Granny $init); + an implementation that does if (!$init instanceof Data) throw... the only difference is your're using a property to pass the argument, which is really quite pointless, IMHO \$\endgroup\$ Apr 24, 2015 at 12:29
  • \$\begingroup\$ What I see Base is for an algortithm skeleton, subclasses methods fill the holes, 1. If work can be done when creating new object from it then constructor shoud not be final 2. If not then you shoud save somewhere the reference of constructor args. What you exactly want makes no sense. \$\endgroup\$
    – cske
    Apr 24, 2015 at 12:43
  • \$\begingroup\$ @elias-van-ootegem i've updated my answer \$\endgroup\$
    – cske
    Apr 24, 2015 at 18:26
  • \$\begingroup\$ @cske: Your alternative suggestion (overriding the constructor) is a breach of the Liskov principle. PHP allows it, but most languages wouldn't. A constructor, just like any other method, should conform to the inherited contract \$\endgroup\$ Apr 27, 2015 at 5:11
0
\$\begingroup\$

As I read it, using this in the super class:

final public function __construct(Granny $init = null)

followed by this in the subclass:

final public function __construct(Dad $init = null)

where Dad is a subtype of Granny is a violation of the Liskov substitution principle (also from the SOLID principles mentioned in the other answer), since it states that preconditions cannot be strengthened in a subtype (see the principle here).

I am not saying that it is obvious in your example, but it seems to be an edge case that can be really hard to get right from the perspective of the PHP developers (and IMHO they have bigger fishes to fry).

\$\endgroup\$
4
  • \$\begingroup\$ All children of the Granny base-class are fully compliant, they only add too the base-class, thus any child will comply with the inherited contract, and no pre-/postconditions will be violated. Dad is a Granny, their mitochondrial DNA (ie contract) is the same, though Dad only knows how to throw a curve-ball \$\endgroup\$ Jul 25, 2013 at 8:20
  • \$\begingroup\$ If you change __construct to accept only Dad in stead of Granny, you're restricting the the domain that $init can belong to. Maybe you think Dad is a great Granny that throws great curveballs made of DNA and stuff, but that doesn't change anything. (What are you smoking, by the way?) \$\endgroup\$ Jul 25, 2013 at 20:27
  • \$\begingroup\$ I can't change the __construct, because it's final public function __construct(Granny $dep = null) in the abstract Base class. If I pass a Dad to that constructor, covariance does apply... what's so hard to understand about that? \$\endgroup\$ Jul 26, 2013 at 7:05
  • \$\begingroup\$ BTW: I don't know what you were thinking but what you read into my question isn't what I was asking, plus: your two method signatures will result in a fatal error: you can't override a final method in a subclass \$\endgroup\$ Sep 4, 2013 at 8:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.