-
Notifications
You must be signed in to change notification settings - Fork 150
Conversation
The main reason Can you demonstrate some of the use cases you have for this change, please? |
Yes sure, for a example a class RedirectResponse
{
use ResponseTrait;
public function __construct($redirectUrl)
{
$this->stream = new Stream('php://memory', 'wb+');
$this->statusCode = 302;
$this->headers = ['Location: '.$redirectUrl];
}
} This is still not perfect, as a lot has to be done manually (e.g. setting the reason phrase) but it offers an other approach for custom responses, in addition to the static generator methods from the develop-branch. |
@mrgrain I think inheritance it's better in your example. Diactotors Response and your example RedirectResponse share the same semantic field |
@Maks3w Yes and no. I also would prefer inheritance. The issue I have with Diactotors Response is it's signature. Especially the stream string/resource as first parameter is something I really don't care about as a package user. I know this is an important and interesting field to have, but in most cases I actually would consider the used stream an implementation detail. |
@mrgrain One interesting fact of PHP is that constructors can differ in child classes. This allows you to do the following: class Response implements ResponseInterface
{
public function __construct($body = 'php://memory', $status = 200, array $headers = [])
{
/* initialize */
}
}
class Redirect extends Response
{
public function __construct($redirectUrl, $status = 302)
{
parent::__construct(
'php://memory',
$status,
[
'location' => [$redirectUrl],
]
);
}
} PHP will raise no errors in the above; constructors are exempt from the parameter inheritance rules present for any other method. The problem that does arise, however, is that we've defined all properties in our messages as private. This is to enforce encapsulation. It does mean, however, that if you want to extend the messages, you cannot directly manipulate properties without first redefining them in your extending class — and this may lead to odd issues later down the line. And this, @Maks3w, is why inheritance may not be the correct solution for all use cases. That said: the example I provided above already works, and doesn't require having a trait. Knowing this, @mrgrain, is the PR necessary? |
@mrgrain BTW, you might be interested in the following:
|
I'm going to close this, as I think I've sufficiently demonstrated that you can create custom extensions trivially, and I'm not convinced of the need for extracting the response-specific functionality into a trait at this time. |
Yes, that's perfectly fine for me :) I amazingly didn't know about the possibility to overload constructors. Learned something new today. Thanks for the documentation links. They are looking very promising. |
@mrgrain Those will be in the upcoming 1.1 version; I'll likely tag it in the next couple of days. :) |
Moved related methods to a ResponseTrait
Using ResponseTrait, a class will implement the ResponseInterface
This basically allows for creating custom responses without the need to use the default Response constructor.