Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of late static binding to allow decorator extension #78

Merged
merged 8 commits into from
Dec 14, 2018
Merged

Make use of late static binding to allow decorator extension #78

merged 8 commits into from
Dec 14, 2018

Conversation

trvrnrth
Copy link
Contributor

@trvrnrth trvrnrth commented Dec 4, 2018

We would like to extend the response decorator in order to add additional domain-specific helpers. Unfortunately this is not possible at present as the current helpers do not make use of late static binding.

Using the response as an example, the current helpers take the basic format of:

/**
 * @return Response
 */
public function withX(): ResponseInterface
{
    $response = $this->response->withY();
    return new Response($response, $this->streamFactory);
}

In order to allow extension it would be nice if these made use of late static binding to type hint and create the new decorated response as follows:

/**
 * @return static
 */
public function withX(): ResponseInterface
{
    $response = $this->response->withY();
    return new static($response, $this->streamFactory);
}

@coveralls
Copy link

coveralls commented Dec 4, 2018

Coverage Status

Coverage remained the same at 99.275% when pulling 7586b67 on blubolt:late-static-binding into ff98230 on slimphp:master.

@akrabat akrabat requested a review from l0gicgate December 13, 2018 08:12
Copy link
Member

@l0gicgate l0gicgate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @akrabat if you want to merge

@l0gicgate
Copy link
Member

@trvrnrth please fix conflicts that arose from merging your other PRs and I’ll merge.

@trvrnrth
Copy link
Contributor Author

@l0gicgate Resolved.

Thanks all for taking the time to get these in.

@l0gicgate l0gicgate added this to the 0.7 milestone Dec 14, 2018
@l0gicgate l0gicgate merged commit 83ff8a0 into slimphp:master Dec 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants