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

LSP violation #122

Merged
merged 3 commits into from
Aug 11, 2019
Merged

LSP violation #122

merged 3 commits into from
Aug 11, 2019

Conversation

mapogolions
Copy link
Contributor

Suppose we have a function f that perfectly works with some domain of values called A.
[Domain A/input] -> f -> [Codomain A/output]
At the same time, we have another function g that works fine with another domain of values called B.
[Domain B/input] -> g -> [Codomain B/output]
The function f can safely replace the function g if and only if the domain B is a subset of the domain A && the codomain A is a subset of the codomain B.
Let's look at the following signatures

  • Headers::setHeader(string $name, ...) - f(domain A is string type)
  • Message::withHeader($name, ...) - g(domain B is any type)

When we do something like this $response->withHeader($any, ...) we violate the above principle. The specified method call is transformed to $this->headers->setHeader($any, ...).
The domain A (string type) is less than the domain B (any type). For this reason, unnecessary static methods had to be implemented causing a problem.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.001%) to 99.73% when pulling 5f8a1fb on mapogolions:enhancement/LSP-violation into 0b3be28 on slimphp:master.

@l0gicgate l0gicgate added this to the 0.5 milestone Aug 10, 2019
@l0gicgate
Copy link
Member

I think we may have gotten carried away with the idea of strong typing every method as much as possible. I agree that we are violating said principle. @adriansuter I'm fine with what is being proposed. Please review before I merge.

@l0gicgate l0gicgate merged commit 78ff6b8 into slimphp:master Aug 11, 2019
@mapogolions mapogolions deleted the enhancement/LSP-violation branch August 11, 2019 20:59
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.

4 participants