Skip to main content

You are not logged in. Your edit will be placed in a queue until it is peer reviewed.

We welcome edits that make the post easier to understand and more valuable for readers. Because community members review edits, please try to make the post substantially better than how you found it, for example, by fixing grammar or adding additional resources and hyperlinks.

Required fields*

21
  • 2
    I think both your and the question's examples are LSP violations if it is not part of the interface contract that invalid arguments throw in that way. If eat is defined as "Instance eats the specified food or throws InvalidArgumentException if it's incompatible" then we're good. If it's merely "Instance eats the specified food" then we're not good, but also the Animal interface was poorly designed. Commented May 8, 2018 at 8:49
  • @Phoshi: I was already drafting an update to the updated example (see my answer now). The difference is that in OP's initial example, the exception was related to the language used (which allows typeless parameters - thus opening the door to unexpected types being passed - thus making it acceptable to throw on unusable types, in my opinion). However, in the revised example, he's now working with a typed parameter (which in and of itself displays inheritance on top of that), which does significantly change the violation here. I hope you agree with my updated answer. Commented May 8, 2018 at 8:54
  • @Phoshi: Maybe a better reply after re-reading your comment: Regardless of the contract definition, if no meaningful result can be achieved; then an exception is the only reasonable outcome. LSP or not, if there's no other option than throwing an exception, throwing an exception can't be wrong or in violation of anything. Commented May 8, 2018 at 11:16
  • I think the origin example also break the LSP because the original contract TransformerInterface::transform is considered to handle "everything", like PHP's var_dump function. However the TagTransformer::transform only can handle "string". which should be more appropriate as TagTransformer::transform(string $origin). no matter there's typehint or not, I always fell it's a code smell of type checking for parameters, no matter it's "typeof" or "is_xxx" Commented May 8, 2018 at 11:53
  • @chrisyue: Those are valid concerns (I don't like untyped parameters for exactly this reason), but that's not really what LSP is about. Having to type check is a logical consequence of not enforcing a type to begin with. If you don't like type checking, then enforce a type. Problem solved. Type checking of typed variables (e.g. Base b = new Derived(); if(b is Derived) { HandleDerived(d); }) is more often than not an abuse of polymorphism; and I suggest avoiding this altogether to prevent future code smells and LSP violations. Commented May 8, 2018 at 11:59