Skip to content

Instantly share code, notes, and snippets.

@taylorotwell
Last active March 9, 2018 16:36
Show Gist options
  • Save taylorotwell/ee2f782aec59aa53863fd09c8e47f304 to your computer and use it in GitHub Desktop.
Save taylorotwell/ee2f782aec59aa53863fd09c8e47f304 to your computer and use it in GitHub Desktop.
<?php
class Decorator
{
public function __construct($target)
{
$this->target = $target;
}
public function __call($method, $parameters)
{
return $this->target->{$method}(...$parameters);
}
}
class Target
{
}
class Consumer
{
public function doesWork(Target $target)
{
};
}
// Doesn't work becuase type-hints are a waste of time and lead to poorly coded, brittle systems... :)
$consumer->doesWork(new Decorator(new Target));
@edsonmedina
Copy link

Have you heard of interfaces?

doesWork method should expect an interface, not a specific class.

@majkel89
Copy link

interfaces ;-) simpleclass Decorator extends Target {} should work ;-) That is the basics of OOP

@greg-1-anderson
Copy link

I think that @taylorotwell is looking for template classes here. The point of the Decorator class is that, as written, it can be used to decorate any php class. Making Decorator implement an interface breaks this. Typehinting also breaks the ability to pass null as the value of the parameter. Some feel these added restrictions are features, as they increase the formality of the code. They do break some use cases, though.

@tpavlek
Copy link

tpavlek commented Feb 16, 2017

@majkel89 but that means you can only decorate objects you own.

@platedodev
Copy link

@edsonmedina I bet he did. He's Laravel's creator.

@jroszkiewicz
Copy link

Taylor can't into OOP.

@taha
Copy link

taha commented Feb 16, 2017

Typehinting also breaks the ability to pass null as the value of the parameter.

@greg-1-anderson not true 😉

class Consumer
{
    public function doesWork(Target $target = null)
    {
        var_dump($target);
    }
}

$consumer = new Consumer;

// class Target#2 (0) {
// }
$consumer->doesWork(new Target);

// passes => NULL
$consumer->doesWork();

@afquinterog
Copy link

i bet he knows all about OOP, he is trying to do something really generic and type-hints sometimes makes it very specific.

@RemiCollin
Copy link

Generated proxy classes solve this nicely. https://github.com/Ocramius/ProxyManager

@jlem
Copy link

jlem commented Feb 23, 2017

// Doesn't work becuase type-hints are a waste of time and lead to poorly coded, brittle systems... :)

Sorry buddy, I love you and all that, but this statement is way out in left field.

The problem is you haven't added a contract to that system. It's the contracts that matter in OOP, not the objects. You don't type hint objects, you type hint the behavioral contracts. doesWork() needs to have a contract for the type of work it is delegating to the target, and therefore the target or anything in a decorator chain needs to respect that contract.

The motivation is pretty simple: you want to guarantee that, regardless of the type of object you pass into doesWork(), that whatever messages doesWork() sends to it's $target argument, the code won't blow up.

I've been working heavily in JavaScript as my full time job over the last 18 months or so. I went from type hinted OO PHP to duck typed JavaScript, and the lack of type hinting is un-ignorably more painful in JS. Uncaught TypeError: foo.bar is not a function errors are all too common in JS because of the lack of typing. You pass in an object but have absolutely no way of knowing if that object is compatible with the function that's consuming it unless you use the source code in that client function to find out, and then also dive into the object itself, to see if they match up.

You don't have that problem when leveraging nominal typing. You simply look at the argument on the client, see the type hint, then look at the service code and see if it's a type of that object. If it's not, you know it won't work by virtue of that information. It saves massive amounts of time because it makes the code easier to reason about, and makes the code MUCH more stable. It also lets you write far simpler and far fewer tests - remember - the best tests are the ones you don't even have to write. When you have duck typing, you have to write additional tests to cover the safety that type hints give you for free.

But speaking of "poorly coded and brittle", that's EXACTLY what this code is:

return $this->target->{$method}(...$parameters);

Dynamic method calls are slow (compiler hates them), they're harder to debug, and they're more prone to unexpected, wonky behavior. This is simply a lazy way to write a decorator, and the issue you're having is because you're trying to write a generic decorator rather than something built for a specific purpose, which could then have a specific behavioral contract, that would alleviate all of these issues.

If you don't want to use type hints for Laravel's plumbing, great. Meanwhile I'll continue to make my apps easier to read, easier to understand, easier to debug, more stable, and easier to test by using type hints to describe behavioral contracts ;)

@nicholasruunu
Copy link

A decorator need to implement the same interface as the origin.
The problem with __call (among many) is that it can't adhere to an interface.
What could solve this would be another magic like __proxy

private function __proxy() {
    return $this->origin;
}

Where every method missing from Decorator would be proxied to an origin (Target) and it would adhere to the interface.
Although, this is just to save on keystrokes, you can still do it.

@Ocramius
Copy link

@nicholasruunu that's already provided by the userland library mentioned above

@Potherca
Copy link

Potherca commented Jun 3, 2017

// Doesn't work becuase type-hints are a waste of time and lead to poorly coded, brittle systems... :)

I'm just gonna grammar nazi this and point out that "becuase" is spelled wrong.

Also, you seem to have misspelled "type-hints" as that is not how you spell "idiots" ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment