From 24d127e40a3d20c5db07d6e3664ae67973254698 Mon Sep 17 00:00:00 2001 From: Jamie McKernan Date: Fri, 25 Aug 2023 20:33:17 +0100 Subject: [PATCH 1/3] Add example code and tests for next Liskov --- .../solid/liskov/detailed/exceptions.py | 24 ++++++++++ .../solid/liskov/tests/exceptions_test.py | 47 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 src/design_principles/solid/liskov/detailed/exceptions.py create mode 100644 src/design_principles/solid/liskov/tests/exceptions_test.py diff --git a/src/design_principles/solid/liskov/detailed/exceptions.py b/src/design_principles/solid/liskov/detailed/exceptions.py new file mode 100644 index 0000000..b2a000e --- /dev/null +++ b/src/design_principles/solid/liskov/detailed/exceptions.py @@ -0,0 +1,24 @@ +from pathlib import Path + + +class FileError(Exception): + pass + + +class MissingFileError(FileError): + pass + + +class SystemFileReader: + def open_file(self, path: Path) -> bytes: + raise FileError() + + +class WindowsFileReader(SystemFileReader): + def open_file(self, path: Path) -> bytes: + raise MissingFileError() + + +class MacOSFileReader(SystemFileReader): + def open_file(self, path: Path) -> bytes: + raise ValueError() diff --git a/src/design_principles/solid/liskov/tests/exceptions_test.py b/src/design_principles/solid/liskov/tests/exceptions_test.py new file mode 100644 index 0000000..26f24dd --- /dev/null +++ b/src/design_principles/solid/liskov/tests/exceptions_test.py @@ -0,0 +1,47 @@ +from pathlib import Path + +import pytest + +from src.design_principles.solid.liskov.detailed.exceptions import ( + FileError, + MacOSFileReader, + SystemFileReader, + WindowsFileReader, +) + + +def test_normal_exceptions_are_caught() -> None: + # given + reader = SystemFileReader() + + try: + # when + reader.open_file(Path("test.txt")) + except FileError: + # then + assert True + + +def test_subclass_follows_liskov() -> None: + # given + reader = WindowsFileReader() + + try: + # when + reader.open_file(Path("test.txt")) + except FileError: + # then + assert True + + +@pytest.mark.xfail(reason="This test demonstrates an anti-pattern.") +def test_subclass_breaks_liskov() -> None: + # given + reader = MacOSFileReader() + + try: + # when + reader.open_file(Path("test.txt")) + except FileError: + # then + assert True From 9a8a0ae0032ebfbb5a375f7ead0078682b284b6a Mon Sep 17 00:00:00 2001 From: Jamie McKernan Date: Fri, 25 Aug 2023 20:59:15 +0100 Subject: [PATCH 2/3] Add base tutorial This will probably need a proof-read when I'm not as tired. --- .../solid/liskov/detailed/Exceptions.md | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 src/design_principles/solid/liskov/detailed/Exceptions.md diff --git a/src/design_principles/solid/liskov/detailed/Exceptions.md b/src/design_principles/solid/liskov/detailed/Exceptions.md new file mode 100644 index 0000000..55797be --- /dev/null +++ b/src/design_principles/solid/liskov/detailed/Exceptions.md @@ -0,0 +1,101 @@ +# Detailed Liskov Substitution + +_(X minute read)_ + +## Structure + +| File | Description | +| ----------- | ----------- | +| [`exceptions.py`](exceptions.py) | Code example containing an anti-pattern. | +| [`../tests/exceptions_test.py`](../tests/exceptions_test.py) | Unit tests to show code in action. | + +## Exceptions + +When inheriting from a class, all exceptions raised by a method should match, or be a +subtype of the original exception. + +This is because if you have code that is in a try/except block, the exception it is +expecting to catch could be a generic one and so if you raise that exception, or a +subtype of that exception then everything will work fine. But if you raise a different +type of exception then the error will slip through your try/except and crash your +program. + +The short example should make this easy to understand. + +### The example + +Our simple example is about a file system class that can read the files on a computer. +As you can see, we have three types: + + - A generic `SystemFileReader` + - A reader specific to Microsoft Windows: `WindowsFileReader` + - And a reader specific to Apple Mac OSX: `MacOSFileReader` + +The Windows and MacOS classes are a subclass of the generic system, so any errors +that these subclasses raise should match or be a subclass of the errors that are +raised in the corresponding methods of the parent class. + +Let's look at our tests to see this in action: + +```python +def test_normal_exceptions_are_caught() -> None: + # given + reader = SystemFileReader() + + try: + # when + reader.open_file(Path("test.txt")) + except FileError: + # then + assert True +``` + +This first test demonstrates how some typical code might work. We are using the +generic class, and we know that a `FileError` may occur, so we have some code to +check for this and compensate for it. The test passes fine. + +```python +def test_subclass_follows_liskov() -> None: + # given + reader = WindowsFileReader() + + try: + # when + reader.open_file(Path("test.txt")) + except FileError: + # then + assert True +``` + +Our next test uses the Windows class, and if you look at the original code, this +actually raises a `MissingFileError`. You may have also noticed that the code in +this test is the exact same as the first test. This is because if Liskov is followed +correctly, you _should_ be able to swap these classes and subclasses out without any +issues! + +And so because `MissingFileError` is a subclass of `FileError`, this test also +passes fine. + +Our problem comes to when we look at the MacOS test: + +```python +def test_subclass_breaks_liskov() -> None: + # given + reader = MacOSFileReader() + + try: + # when + reader.open_file(Path("test.txt")) + except FileError: + # then + assert True +``` + +The code again looks the same, but the test fails because the MacOS code raises a +`ValueError` which is not a subclass of `FileError`. This means that the exception +slips through our try/except block and crashes our program! + +## Conclusion + +This quick guide should give you an idea for how exceptions can slip through our +client code if they do not also follow the LSP. \ No newline at end of file From a75909e9bb97e0c0c8db1ac87cb9b0f5bfd1be2f Mon Sep 17 00:00:00 2001 From: Jamie McKernan Date: Sun, 27 Aug 2023 20:29:04 +0100 Subject: [PATCH 3/3] Clean up tutorial There were definitely symptoms of fatigue that needed sorting out! --- .../solid/liskov/detailed/Exceptions.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/design_principles/solid/liskov/detailed/Exceptions.md b/src/design_principles/solid/liskov/detailed/Exceptions.md index 55797be..70a2d99 100644 --- a/src/design_principles/solid/liskov/detailed/Exceptions.md +++ b/src/design_principles/solid/liskov/detailed/Exceptions.md @@ -6,13 +6,13 @@ _(X minute read)_ | File | Description | | ----------- | ----------- | -| [`exceptions.py`](exceptions.py) | Code example containing an anti-pattern. | +| [`exceptions.py`](exceptions.py) | Code example containing a pattern and anti-pattern. | | [`../tests/exceptions_test.py`](../tests/exceptions_test.py) | Unit tests to show code in action. | ## Exceptions When inheriting from a class, all exceptions raised by a method should match, or be a -subtype of the original exception. +subtype of the original exception raised in the parent class. This is because if you have code that is in a try/except block, the exception it is expecting to catch could be a generic one and so if you raise that exception, or a @@ -20,11 +20,11 @@ subtype of that exception then everything will work fine. But if you raise a dif type of exception then the error will slip through your try/except and crash your program. -The short example should make this easy to understand. +The following short example should make this easy to understand. ### The example -Our simple example is about a file system class that can read the files on a computer. +Our example is about a file system class that can read the files on a computer. As you can see, we have three types: - A generic `SystemFileReader` @@ -70,13 +70,13 @@ def test_subclass_follows_liskov() -> None: Our next test uses the Windows class, and if you look at the original code, this actually raises a `MissingFileError`. You may have also noticed that the code in this test is the exact same as the first test. This is because if Liskov is followed -correctly, you _should_ be able to swap these classes and subclasses out without any +correctly, you _should_ be able to swap these classes and subclasses without any issues! And so because `MissingFileError` is a subclass of `FileError`, this test also passes fine. -Our problem comes to when we look at the MacOS test: +Our problem comes when we look at the MacOS test: ```python def test_subclass_breaks_liskov() -> None: