commit 75d395b707aabcc5d49e1164b59a8343e3459eaa
parent 29695afad439f8a2fb213c84c2b25a96b7640419
Author: Sebastiano Tronto <sebastiano@tronto.net>
Date: Tue, 30 May 2023 16:51:19 +0200
Added questions, comments and alternative version
Diffstat:
| M | README.md | | | 69 | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- |
| A | thermostat-alt.py | | | 50 | ++++++++++++++++++++++++++++++++++++++++++++++++++ |
2 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/README.md b/README.md
@@ -1 +1,68 @@
-Just an exercise on TDD.
+# Thermostat
+
+This is an exercise for TDD. Check the git log to see the
+RED-GREEN-REFACTOR process.
+
+## Questions and comments
+
+### Question(s)
+
+One of the points of the exercise is to find a way to "mock" the
+system time, so we can inject the dependency and test the program
+regardless of the current time. But I actually had to look up how
+to get the current time from the system in python. However, I had
+no way to properly test it if I were to follow the TDD workflow
+strictly.
+
+**How can I test that the system library to get the current time
+actually works, and that I am calling it correctly?**
+
+This is something I seriously would like to test in real life. After
+writing down this question I recalled reading about testing external
+libraries - maybe in *Clean Code* by Uncle Bob, or in one of his
+videos. He mentions writing *learning tests* to do just this, if I
+am not mistaken.
+
+My follow-up question then is: what do I do with these learning
+tests? Do I keep them in production code, remove them before merging
+to master, or just throw them away as soon as I learn what I need?
+
+### Comment(s)
+
+I do not quite like how I injected the dependency for the system
+time. I think the reason is that this global state is hidden behind
+some instance property in the `Thermostat` class (i.e. the method
+`GetCurrentTime()`). Mocking this state requires fiddling with an
+object's internal state. I think this is something I dislike about
+OOP in general.
+
+After some reflection, I think a better way of mocking this state
+would something along these lines:
+
+```
+def GetCurrentRequiredTemperatureAtTime(self, time):
+ # logic for getting temp here
+
+def GetCurrentRequiredTemperature(self):
+ time = int(datetime.now().strftime("%H"))
+ return self.GetCurrentRequiredTemperatureAtTime(time)
+```
+
+This also removes the need for the mocking class with its overrides.
+I added this version in the `thermostat-alt.py` file.
+
+But then I asked myself: if I dislike how state is managed here and
+prefer having an explicit parameter, why not also getting rid of
+the whole state of the `Thermostat` class and using something like
+
+```
+def GetCurrentRequiredTemperatureAtTimeWithState(time, state):
+ # logic here, current set points given in state
+```
+
+But then I answered myself that the OOP approach works better here.
+We don't care what the `state` variable holds inside, so we might
+just as well leave it at the `Thermostat` class to manage it.
+
+But for external dependencies, I think I like the explicit parameter
+version more.
diff --git a/thermostat-alt.py b/thermostat-alt.py
@@ -0,0 +1,50 @@
+import unittest
+from datetime import datetime
+
+class Thermostat:
+
+ def __init__(self):
+ self.setPoints = []
+
+ def GetCurrentRequiredTemperature(self):
+ time = int(datetime.now().strftime("%H"))
+ return self.GetCurrentRequiredTemperatureAtTime(time)
+
+ def GetCurrentRequiredTemperatureAtTime(self, time):
+ passedSetPoints = [sp for sp in self.setPoints if sp[0] <= time]
+ lastPassedSetPoint = max(passedSetPoints, default = (0, 10), key = lambda sp: sp[0])
+ return lastPassedSetPoint[1]
+
+ def AddSetPoint(self, hour, temperature):
+ self.setPoints.append((hour, temperature))
+
+class ThermostatTests(unittest.TestCase):
+
+ def test_GivenNewThermostat_ThenRequiredTemperatureIs10(self):
+ thermostat = Thermostat()
+ currentRequiredTemperature = thermostat.GetCurrentRequiredTemperature()
+ self.assertEqual(currentRequiredTemperature, 10)
+
+ def test_GivenIAddedASetPoint_WhenTimeIsAfterSetPoint_ThenRequiredTemperatureIsChanged(self):
+ hour = 10
+ requiredTemperature = 30
+ thermostat = Thermostat()
+ thermostat.AddSetPoint(hour, requiredTemperature)
+ self.assertEqual(thermostat.GetCurrentRequiredTemperatureAtTime(hour), requiredTemperature)
+
+ def test_GivenIAddedSetPoint_WhenTimeIsBeforeSetPoint_ThenRequiredTemperatureNotChanged(self):
+ hour = 7
+ requiredTemperature = 25
+ thermostat = Thermostat()
+ oldRequiredTemperature = thermostat.GetCurrentRequiredTemperatureAtTime(hour-1)
+ thermostat.AddSetPoint(hour, requiredTemperature)
+ newRequiredTemperature = thermostat.GetCurrentRequiredTemperatureAtTime(hour-1)
+ self.assertEqual(oldRequiredTemperature, newRequiredTemperature)
+
+ def test_GivenTwoSetPoints_WhenTimeIsBetweenThem_ThenRequiredTemperatureIsFirst(self):
+ hour1, hour2 = 12, 15
+ temp1, temp2 = 55, 75
+ thermostat = Thermostat()
+ thermostat.AddSetPoint(hour1, temp1)
+ thermostat.AddSetPoint(hour2, temp2)
+ self.assertEqual(thermostat.GetCurrentRequiredTemperatureAtTime(14), 55)
