Liskov: Unexpected Exceptions
Keep on Learning!
If you liked what you've learned so far, dive in! Subscribe to get access to this tutorial plus video, code and script downloads.
With a Subscription, click any sentence in the script to jump to that part of the video!
Login SubscribeLet's jump into our first example where we learn how we can violate the Liskov principle! And... maybe more importantly, why... that's not such a great idea.
Creating a new Scoring Factor
In the src/Scoring/
directory, create a new scoring factor class called PhotoFactor
... and make it implement the ScoringFactorInterface
. We'll finally fulfill the change request we received earlier: to add a scoring factor that reads the images for each sighting.
// ... lines 1 - 6 | |
class PhotoFactor implements ScoringFactorInterface | |
{ | |
// ... lines 9 - 21 | |
} |
Thanks to our work with the open-closed principle, we can now add this scoring factor without touching SightingScorer
. And to be extra cool, thanks to this tagged_iterator
thing in services.yaml
, the new PhotoFactor
service will be instantly passed into SightingScorer
. Yay!
In PhotoFactor
, go to Code -> Generate - or Command + N on a Mac - and select "Implement Methods" to generate the score()
method. Inside, I'll paste some code.
// ... lines 1 - 8 | |
public function score(BigFootSighting $sighting): int | |
{ | |
if (count($sighting->getImages()) === 0) { | |
throw new \InvalidArgumentException('Invalid BigFootSighting, it should have at least one photo'); | |
} | |
$score = 0; | |
foreach ($sighting->getImages() as $image) { | |
$score += rand(1, 100); // todo analyze image | |
} | |
return $score; | |
} | |
// ... lines 22 - 23 |
This is pretty simple: we loop over the images... and pretend that we're analyzing them in some super advanced way. Shh, don't tell our users. Oh, and if there are no images for this sighting, we throw an exception.
Cool! Let's try it. Go back to our homepage, click to add a new post and fill in some details. I'll leave images empty for simplicity. And... ah! A 500 error! That's our new exception! We broke our app! And it broke because we violated Liskov's principle! She tried to warn us!
Our new scoring factor class - or subtype - to use the more technical word, just did something unexpected: it threw an exception!
The Ugly Work-Around
One way to fix this, which might seem silly... but there's a reason we're doing this... is to add some conditional code inside of SightingScorer
. If PhotoFactor
doesn't like sightings with zero images, let's just skip that factor when that happens!
Inside the foreach
, if ScoringFactor
is an instanceof PhotoFactor
and count of $sighting->getImages()
equals zero, then continue
.
// ... lines 1 - 5 | |
use App\Scoring\PhotoFactor; | |
// ... lines 7 - 8 | |
class SightingScorer | |
// ... lines 11 - 21 | |
public function score(BigFootSighting $sighting): BigFootSightingScore | |
{ | |
// ... line 24 | |
foreach ($this->scoringFactors as $scoringFactor) { | |
// LSP violation and also OCP violation | |
if ($scoringFactor instanceof PhotoFactor && count($sighting->getImages()) === 0) { | |
continue; | |
} | |
// ... lines 30 - 31 | |
} | |
// ... lines 33 - 34 | |
} | |
} |
In addition to this not being the best way to fix this - more on that in a minute - this also violates the open-closed principle. But... it does fix things: if we resubmit the form... our app works again!
Exceptions are a "Soft" Part of an Interface
But... let's back up. Open ScoringFactorInterface
. Unlike argument types and return types, there's no way in PHP to codify whether or not a method should throw an exception or which types of exceptions should be used. But this can, at least, be described in the documentation above the method... which we totally skipped!
Let's fill that in. We don't need the @return
or @param
because they're redundant... unless we want to add some more information about their meaning. I'll add a quick description... and then let's be very clear about the exception behavior we expect:
This method should not throw an exception for any normal reason.
// ... lines 1 - 6 | |
interface ScoringFactorInterface | |
{ | |
/** | |
* Return the score that should be added to the overall score. | |
* | |
* This method should not throw an exception for any normal reason. | |
*/ | |
public function score(BigFootSighting $sighting): int; | |
} |
In the real-world, if a method is allowed to throw an exception when some expected situation happens, you would typically see an @throws
that describes that. And if you don't see that, you can assume that you are not allowed to throw an exception for any normal situation.
Our Class Behaves Unexpectedly
Anyways, now that we've clarified this, it's easy to see that our PhotoFactor
breaks Liskov's principle: PhotoFactor
behaves in a way that the class that uses it - SightingScorer
, sometimes called the "client class" - was not expecting. That "bad behavior" caused us to need to hack in this code to get it to work.
Another way to think about it, which explains why this is called Liskov's substitution principle, is that, if any of our code relies on a ScoringFactorInterface
object - like DescriptionFactor
- we could not "replace" or "substitute" that object for our PhotoFactor
without breaking things.
If this substitution aspect doesn't make complete sense yet, don't worry. Our next example will illustrate it even better.
instanceof Checks Indicate Liskov Violation
So: we violated Liskov's principle by throwing an exception. And then, I lazily worked around the problem by adding some instanceof
code to SightingScorer
... to literally work "around" the problem.
When you have an instanceof
conditional like this, it's often a signal that you're violating Liskov because it means that you have a specific implementation of a class or interface that is behaving differently than the rest... which you then need to code for.
So let's remove this: take out the if statement and let's even go clean out the extra use
statement on top.
// ... lines 1 - 8 | |
class SightingScorer | |
{ | |
// ... lines 11 - 20 | |
public function score(BigFootSighting $sighting): BigFootSightingScore | |
{ | |
$score = 0; | |
foreach ($this->scoringFactors as $scoringFactor) { | |
$score += $scoringFactor->score($sighting); | |
} | |
return new BigFootSightingScore($score); | |
} | |
} |
Now that we've clarified that the score()
method should not throw an exception in normal situations, the real fix is... kinda obvious: stop throwing the exception! Replace the exception with return 0
.
// ... lines 1 - 6 | |
class PhotoFactor implements ScoringFactorInterface | |
{ | |
public function score(BigFootSighting $sighting): int | |
{ | |
if (count($sighting->getImages()) === 0) { | |
return 0; | |
} | |
// ... lines 14 - 20 | |
} | |
} |
That's it. The class now acts like we expect: no surprises.
By the way, all of this does does not mean that it is illegal for our score()
method to ever throw an exception. If the method, for example, needed to query a database... and the database connection was down... then yeah! You should totally throw an exception! That is an unexpected situation. But for all the, expected, normal cases, we should follow the rules of our parent class or interface.
Next let's look at one more example of Liskov's principle where we create a subclass of an existing class... then secretly substitute it into our system without breaking anything. Liskov would be so proud!