15
\$\begingroup\$

I'm continuing to work on the Head First Design Patterns book in an effort to become a more efficient and better Python programmer. Code review for the Strategy pattern in Chapter 1 is here with helpful explanations from the community.

I've tried to implement the Observer design pattern from chapter 2 below in Python. In the book all the examples are in Java. I'm sure there are more idiomatic Python things that I can do to this code. In the first example in this chapter we implement the Subject so that it pushes data out to the individual observers. In my code I wanted the Subject to notify the observers that new data was available, and have the observers pull only the data that each individual observer was interested in. I also tried to unit test instead of just printing output.

Some issues I don't know how to resolve:

  • The Observer abstract class has an abstract method update(), but two other non-abstract methods register_subject(self, subject) and remove_subject(self). So when I implement the concrete observers, it inherits these two methods. Is this a wrong way of using the Abstract Base Class?

  • I've recently started to unit test and have had some trouble formulating my tests (for this particular example I wrote the tests after the code, although I know it's better to write the tests first). A particular example: I test that the Observers are in fact registered with the Subject by looking to see that the observer instance is in the Subject._observer_list, but this list is hidden. Is it ok for the test to be looking there?

  • Any other comments or suggestions on the code?

Code

#!/usr/bin/env python
from abc import ABCMeta, abstractmethod
import unittest
"""Implementation of the Observer pattern from Head First Design Patters
(Chapter 2), using the pull method of passing data from the Subject to the
Observer(s). """
################################################################################
# Abstract classes
################################################################################
class Subject:
 __metaclass__ = ABCMeta
 @abstractmethod
 def register_observer(observer):
 """Registers an observer with Subject."""
 pass
 @abstractmethod
 def remove_observer(observer):
 """Removes an observer from Subject."""
 pass
 @abstractmethod
 def notify_observers():
 """Notifies observers that Subject data has changed."""
 pass
class Observer:
 __metaclass__ = ABCMeta
 @abstractmethod
 def update():
 """Observer updates by pulling data from Subject."""
 pass
 def register_subject(self, subject):
 """Observer saves reference to Subject."""
 self.subject = subject
 def remove_subject(self):
 """Observer replaces Subject reference to None."""
 self.subject = None
class DisplayElement:
 __metaclass__ = ABCMeta
 @abstractmethod
 def display():
 """DisplayElement displays instance data."""
 pass
################################################################################
# Concrete Subject class
################################################################################
class WeatherData(Subject):
 def __init__(self):
 self._observer_list = []
 def register_observer(self, observer):
 """Registers an observer with WeatherData if the observer is not
 already registered."""
 try:
 if observer not in self._observer_list:
 self._observer_list.append(observer)
 observer.register_subject(self)
 else:
 raise ValueError
 except ValueError:
 print "ERROR: Observer already subscribed to Subject!"
 raise ValueError
 def remove_observer(self, observer):
 """Removes an observer from WeatherData if the observer is currently
 subscribed to WeatherData."""
 try:
 if observer in self._observer_list:
 observer.remove_subject()
 self._observer_list.remove(observer)
 else:
 raise ValueError
 except ValueError:
 print "ERROR: Observer currently not subscribed to Subject!"
 raise ValueError
 def notify_observers(self):
 """Notifies subscribed observers of change in WeatherData data."""
 for observer in self._observer_list:
 observer.update()
 def set_measurements(self, temperature, humidity, pressure):
 """Function used to simulate weather station device and send out new
 data."""
 self.temperature = temperature
 self.humidity = humidity
 self.pressure = pressure
 self.notify_observers()
################################################################################
# Concrete Observer/DisplayElement classes
################################################################################
class CurrentConditionDisplay(Observer, DisplayElement):
 """Returns the current temperature and humidity."""
 def update(self):
 self.temperature = self.subject.temperature
 self.humidity = self.subject.humidity
 self.display()
 def display(self):
 print "Current conditions: %.1f F degrees and %.1f %% humidity" %\
 (self.temperature, self.humidity)
class ForecastDisplay(Observer, DisplayElement):
 """Returns the a forcast of the weather based on pressure readings."""
 def __init__(self):
 self.current_pressure = 29.92
 def update(self):
 self.last_pressure = self.current_pressure
 self.current_pressure = self.subject.pressure
 self.display()
 def display(self):
 if self.current_pressure > self.last_pressure:
 print "Improving weather on the way!"
 elif self.current_pressure == self.last_pressure:
 print "More of the same"
 elif self.current_pressure < self.last_pressure:
 print "watch out for cooler, rainy weather"
class StatisticsDisplay(Observer, DisplayElement):
 """Returns the temperature statistics for all readings."""
 def __init__(self):
 self._min_temp = 200
 self._max_temp = 0
 self._temp_sum = 0
 self._num_readings = 0
 def update(self):
 if self.subject.temperature > self._max_temp:
 self._max_temp = self.subject.temperature
 if self.subject.temperature < self._min_temp:
 self._min_temp = self.subject.temperature
 self._temp_sum += self.subject.temperature
 self._num_readings += 1
 self.display()
 def display(self):
 print "Avg/Max/Min temperature = %.1f/%.1f/%.1f" %\
 (float(self._temp_sum/self._num_readings),
 self._max_temp, self._min_temp)
################################################################################
# Unit tests
################################################################################
class TestWeatherData(unittest.TestCase):
 def setUp(self):
 self.weather_data = WeatherData()
 self.current_condition_display = CurrentConditionDisplay()
 self.forecast_display = ForecastDisplay()
 self.statistics_display = StatisticsDisplay()
 def test_register_observers(self):
 # Register observers with subject.
 self.weather_data.register_observer(self.current_condition_display)
 self.weather_data.register_observer(self.forecast_display)
 self.weather_data.register_observer(self.statistics_display)
 # Check registered observers in _observer_list.
 self.assertIn(self.current_condition_display,
 self.weather_data._observer_list)
 self.assertIn(self.forecast_display,
 self.weather_data._observer_list)
 self.assertIn(self.statistics_display,
 self.weather_data._observer_list)
 def test_register_remove_forcast_display(self):
 # Register observer with subject.
 self.weather_data.register_observer(self.forecast_display)
 # Check registered observer in _observer_list.
 self.assertIn(self.forecast_display,
 self.weather_data._observer_list)
 # Remove observer from subject.
 self.weather_data.remove_observer(self.forecast_display)
 # Check observer not in _observer_list.
 self.assertNotIn(self.forecast_display,
 self.weather_data._observer_list)
 def test_remove_nonexistant_display(self):
 # Check removing an observer already not in _observer_list throws a
 # ValueError.
 with self.assertRaises(ValueError):
 self.weather_data.remove_observer(self.forecast_display)
 def test_no_register_display_twice_display(self):
 # Check that adding an observer twice to in _observer_list throws a
 # ValueError.
 with self.assertRaises(ValueError):
 self.weather_data.register_observer(self.forecast_display)
 self.weather_data.register_observer(self.forecast_display)
class TestDisplays(unittest.TestCase):
 def setUp(self):
 self.weather_data = WeatherData()
 self.current_condition_display = CurrentConditionDisplay()
 self.forecast_display = ForecastDisplay()
 self.statistics_display = StatisticsDisplay()
 self.weather_data.register_observer(self.current_condition_display)
 self.weather_data.register_observer(self.forecast_display)
 self.weather_data.register_observer(self.statistics_display)
 def test_set_measurements(self):
 # Setting measurements and making sure they are passed correctly.
 self.weather_data.set_measurements(80, 65, 30.4)
 self.assertEqual(self.current_condition_display.temperature, 80)
 self.assertEqual(self.current_condition_display.humidity, 65)
 self.assertEqual(self.forecast_display.current_pressure, 30.4)
 self.weather_data.set_measurements(82, 70, 29.2)
 self.assertEqual(self.current_condition_display.temperature, 82)
 self.assertEqual(self.current_condition_display.humidity, 70)
 self.assertEqual(self.forecast_display.current_pressure, 29.2)
 self.weather_data.set_measurements(87, 90, 29.2)
 self.assertEqual(self.current_condition_display.temperature, 87)
 self.assertEqual(self.current_condition_display.humidity, 90)
 self.assertEqual(self.forecast_display.current_pressure, 29.2)
if __name__ == '__main__':
 unittest.main()

Output

Current conditions: 80.0 F degrees and 65.0 % humidity
Improving weather on the way!
Avg/Max/Min temperature = 80.0/80.0/80.0
Current conditions: 82.0 F degrees and 70.0 % humidity
watch out for cooler, rainy weather
Avg/Max/Min temperature = 81.0/82.0/80.0
Current conditions: 87.0 F degrees and 90.0 % humidity
More of the same
Avg/Max/Min temperature = 83.0/87.0/80.0
.ERROR: Observer already subscribed to Subject!
...ERROR: Observer currently not subscribed to Subject!
.
----------------------------------------------------------------------
Ran 5 tests in 0.001s
OK
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Jan 26, 2013 at 17:59
\$\endgroup\$

1 Answer 1

17
\$\begingroup\$

A few random comments on pieces of the code:

class Subject:
 __metaclass__ = ABCMeta
 @abstractmethod
 def register_observer(observer):
 """Registers an observer with Subject."""
 pass
 @abstractmethod
 def remove_observer(observer):
 """Removes an observer from Subject."""
 pass
 @abstractmethod
 def notify_observers():
 """Notifies observers that Subject data has changed."""
 pass

Why aren't you defining these methods? It seems to me they'll be the same for all subclasses so you don't need to make them abstract and implement them there.

 def register_observer(self, observer):
 """Registers an observer with WeatherData if the observer is not
 already registered."""
 try:
 if observer not in self._observer_list:
 self._observer_list.append(observer)
 observer.register_subject(self)
 else:
 raise ValueError

Don't throw generic exceptions that might get mistaken for something else. I'd raise Exception("Observer already subscribed to Subject!") or a custom exception class.

 except ValueError:
 print "ERROR: Observer already subscribed to Subject!"

Don't print error messages, just throw exceptions.

 raise ValueError

Certainly don't catch an exception right after throwing it and then throw it again.

The Observer abstract class has an abstract method update(), but two other non-abstract methods register_subject(self, subject) and remove_subject(self). So when I implement the concrete observers, it inherits these two methods. Is this a wrong way of using the Abstract Base Class?

That is a perfectly valid and common way of abstract base classes.

I've recently started to unit test and have had some trouble formulating my tests (for this particular example I wrote the tests after the code, although I know it's better to write the tests first). A particular example: I test that the Observers are in fact registered with the Subject by looking to see that the observer instance is in the Subject._observer_list, but this list is hidden. Is it ok for the test to be looking there?

No, you shouldn't test the internal state of the object. You should test the external actions of the object. Here is a sample of how you should test it:

class MyObserver(Observer):
 def __init__(self):
 self.updated = False
 def update(self):
 self.updated = True
def test_it():
 temperature_data = TemperatureData()
 observer = MyObserver()
 temperature_data.register_observer(observer)
 assert not observer.updated()
 temperature_data.set_measurements(2,4,4)
 assert observer.updated()

But the observer pattern is really not a great fit for python. It is not flexible and doesn't take advantage of python's features. The pattern I use is something like this:

class Signal(object):
 def __init__(self):
 self._handlers = []
 def connect(self, handler):
 self._handlers.append(handler)
 def fire(self, *args):
 for handler in self._handlers:
 handler(*args)

On the Subject I'd do this:

class TemperatureData:
 def __init__(self):
 self.changed = Signal()
 def set_temperaturedata(self, foo, bar):
 ...
 self.changed.fire(self)

Then to hook things up I'd say

display = TemperatureDisplay()
temperature_data.changed.connect(display.update_temperaturedata)

I think this model is simpler. It lets objects emit multiple signals that could be observed, a given object can observer multiple other objects. The TemperatureDisplay object doesn't even know where the data is coming from, it just magically shows up on update_temperaturedata making it easier to test that object in isolation.

answered Jan 26, 2013 at 19:02
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.