New Phone Who Dis?
I have in mind a caller-id feature for PubSub. Good time to do this, when users are few. Bears arrive unexpectedly.
We need a sort of long-range communication in our program, and we have the PubSub object to let us deal with that. We use the facility to enable a dungeon item (model) to announce something that the view needs to deal with, such as the fact that the object is being removed. And we use it to enable one dungeon item to inform others that something has happened. The Button announces that it has been pressed, and the corresponding AppearingConten appears.
Sometimes, the subscriber to an event really only wants to hear from one object, if that one publishes. In the case of the Button, we deal with this with a hack: when a button is pressed, it prepends its name (a string) to the message, and the AppearingContent knows the same name. So the Button says ‘a secret key button_pressed’ and the AppearingContent subscribes to that event string.
That’s rather clearly a hack, and it has the unfortunate property that it isn’t possible to subscribe to all “button_pressed” messages in any reasonable way. So I plan to improve PubSub this morning, as follows:
- There will be an event string and a modifier string associated with each publication and subscription.
- You can subscribe to an event with an empty modifier, in which case you’ll get all the events with that event name, regardless of modifier.
- You can subscribe with a modifier, in which case you’ll only see events with that modifier.
- If you subscribe without a modifier, you’ll see all the events with or without modifier.
- If you publish an event with a modifier, all the callbacks subscribed with that modifier will be called, and so will all the callbacks subscribed with an empty modifier.
So every publish will include a modifier, which may be empty. The subscribe senders can specify a particular modifier, or if they provide an empty modifier, they see all such events.
I haven’t locked in on the calling sequence changes, but I’m leaning strongly toward just adding the modifier as another parameter. Internally, we’ll need a sequence or tiny object to serve as our dictionary keys.
You may be wondering about the use of plain strings as event names and modifiers. You’d be right to do so. Event names, at least, should probably be distinguished via an enum or something similar. Separate issue but it would help avoid typos in event names and subsequent confusion about why something doesn’t work.
It’s time to review PubSub. I think it’s pretty simple:
class Subscription:
def __init__(self, callback, one_shot: bool):
self.callback = callback
self.one_shot = one_shot
class PubSub:
def __init__(self):
self.subscriptions = dict()
def subscribe(self, event, callback, one_shot=False):
subscription = Subscription(callback, one_shot)
try:
self.subscriptions[event].append(subscription)
except KeyError:
self.subscriptions[event] = [subscription]
def subscribe_once(self, event, callback):
self.subscribe(event, callback, one_shot=True)
def publish(self, event, *args, **kwargs):
try:
subscriptions = list(self.subscriptions[event])
for subscription in subscriptions:
subscription.callback(event, *args, **kwargs)
if subscription.one_shot:
self.subscriptions[event].remove(subscription)
except KeyError:
pass
Let’s begin by changing signature on all those methods to add a modifier parameter after the event.
def subscribe(self, event, modifier, callback, one_shot=False):
subscription = Subscription(callback, one_shot)
try:
self.subscriptions[event].append(subscription)
except KeyError:
self.subscriptions[event] = [subscription]
def subscribe_once(self, event, modifier, callback):
self.subscribe(event, '', callback, one_shot=True)
def publish(self, event, modifier, *args, **kwargs):
try:
subscriptions = list(self.subscriptions[event])
for subscription in subscriptions:
subscription.callback(event, *args, **kwargs)
if subscription.one_shot:
self.subscriptions[event].remove(subscription)
except KeyError:
pass
The refactoring has provided empty strings on all the calls. So far so good. I think we can “just” make the dictionary key be a sequence (event,modifier) and most things will work. I think we’ll want another test as well, but let’s do the sequence thing.
def subscribe(self, event, modifier, callback, one_shot=False):
key = (event, modifier)
subscription = Subscription(callback, one_shot)
try:
self.subscriptions[key].append(subscription)
except KeyError:
self.subscriptions[key] = [subscription]
def subscribe_once(self, event, modifier, callback):
self.subscribe(event, '', callback, one_shot=True)
def publish(self, event, modifier, *args, **kwargs):
key = (event, modifier)
try:
subscriptions = list(self.subscriptions[key])
for subscription in subscriptions:
subscription.callback(event, *args, **kwargs)
if subscription.one_shot:
self.subscriptions[key].remove(subscription)
except KeyError:
pass
We are green. Let’s commit: PubSub using event+modifier key. Wild card not yet done.
I think we’d like a wild card test.
def test_two_subscribers_one_wild(self):
called_sub_1 = False
def called_1(event):
nonlocal called_sub_1
called_sub_1 = True
called_sub_2 = False
def called_2(event):
nonlocal called_sub_2
called_sub_2 = True
pub_sub = PubSub()
pub_sub.subscribe('event', 'modified', called_1)
pub_sub.subscribe('event', '', called_2)
pub_sub.publish('event', 'modified')
assert called_sub_1
assert called_sub_2
Fails, not calling sub_2, because we only search for a key with the modifier. We need to search for the event and an empty modifier as well.
So we need to improve this method:
def publish(self, event, modifier, *args, **kwargs):
key = (event, modifier)
try:
subscriptions = list(self.subscriptions[key])
for subscription in subscriptions:
subscription.callback(event, *args, **kwargs)
if subscription.one_shot:
self.subscriptions[key].remove(subscription)
except KeyError:
pass
Let’s extract the try block to a method:
def publish(self, event, modifier, *args, **kwargs):
specific = (event, modifier)
self.execute_callbacks(specific, event, args, kwargs)
general = (event, '')
self.execute_callbacks(general, event, args, kwargs)
def execute_callbacks(self, key, event, args, kwargs):
try:
subscriptions = list(self.subscriptions[key])
for subscription in subscriptions:
subscription.callback(event, *args, **kwargs)
if subscription.one_shot:
self.subscriptions[key].remove(subscription)
except KeyError:
pass
The test is green. Commit: PubSub supports modifier and empty modifier as wildcard.
Fortunately I try to run the game. Dungeon forwards pub sub messages and we need to change this:
def publish(self, event, *args, **kwargs):
self.pub_sub.publish(event, '', *args, **kwargs)
def subscribe(self, event, callback):
self.pub_sub.subscribe(event, '', callback)
def subscribe_once(self, event, callback):
self.pub_sub.subscribe_once(event, '', callback)
To this:
def publish(self, event, modifier, *args, **kwargs):
self.pub_sub.publish(event, modifier, *args, **kwargs)
def subscribe(self, event, modifier, callback):
self.pub_sub.subscribe(event, modifier, callback)
def subscribe_once(self, event, modifier, callback):
self.pub_sub.subscribe_once(event, modifier, callback)
Fortunately, I try again to run the game. Something explodes when I try to pick up an item.
File "/Users/ron/PycharmProjects/dungeon/src/pub_sub.py", line 33, in execute_callbacks
subscription.callback(event, *args, **kwargs)
TypeError: DungeonView.subscribe_to_remove_item.<locals>.callback() missing 1 required positional argument: 'item'
I am fortunate to find the problem. The following Dungeon method did not include the modifier:
def receive_item(self, item):
self.player_inventory.append(item)
content = self.contents_at(self.player_cell)
if item in content:
self.publish('remove_item', '', item)
content.remove(item)
The reason was that I edited the three Dungeon forwarding methods above rather than use ChangeSignature. I’d best check senders of all of them. I do. They are now OK.
Commit: fix defect causing crash on picking up items.
Let’s settle down and reflect.
Reflection
Two issues arose while making this change, both having to do with the forwarding methods in Dungeon. We need tests for that, or something similar.
Oh here is an idea. We want to ensure that PubSub’s methods are replicated as forwarders in Dungeon. So let’s have an abstract class specifying those methods, and inherit it in both PubSub itself and in Dungeon. Then PyCharm and Python should tell us when we get it wrong.
I think I’ll do that while I have the energy.
class PubSubABC(ABC):
@abstractmethod
def subscribe(self, event, modifier, callback, one_shot=False):
pass
@abstractmethod
def subscribe_once(self, event, modifier, callback):
pass
@abstractmethod
def publish(self, event, modifier, *args, **kwargs):
pass
class PubSub(PubSubABC):
...
Now let’s inherit that in Dungeon. I do that and this line gets diagnosed:
def subscribe(self, event, modifier, callback):
self.pub_sub.subscribe(event, modifier, callback)
That is correct, to match exactly it should be:
def subscribe(self, event, modifier, callback, one_shot=False):
self.pub_sub.subscribe(event, modifier, callback, one_shot)
Now the truth is, at least my current truth is, the one_shot flag is a hack and perhaps we should handle that capability better, using a private method in PubSub instead of a magic parameter in subscribe.
Nonetheless, the abstract class idea works perfectly, requiring Dungeon to stay in sync with PubSub.
Let’s rename that abstract class to PubSubProtocol. PyCharm does that willingly. I do love me some PyCharm.
Let’s pause here and sum up.
Summary
We’ve added a “modifier” notion to PubSub, with the meaning that a publication can have a non-empty modifier, and subscribers can subscribe to the corresponding event only if it also has that modifier. Along the way we ran into issues with the forwarding methods in Dungeon. We corrected the defects, but then created an abstract protocol for both PubSub and Dungeon to use to keep the forwarding methods in sync. And it worked, spotting a difference that I had missed.
In the sequel, we should be able to remove the button-pressed publication entirely, instead subscribing to a state change on a button with a known modifier. That will reduce the event space by one, and will also result in fewer messages being sent, not that that is a big deal but it is nice.
I could have avoided the second issue with the forwarding had I used the Change Signature refactoring instead of just typing in the new parameter. My bad. But the first issue just went right over my head until I ran the program. We’ve plugged that hole now but a wiser head than mine would have written a test for the forwarding methods. Me, smart guy that I am, I was sure that I could just type them in and make them work.
And, full credit to me, I did in fact just type them in and make them work. It’s just that after that, with no tests or other connection like our new protocol, changes to the one place did not propagate to the other place unless I remembered to do it.
I don’t know about you, but betting on me to remember something, well, you want to get favorable odds, because I’m not about remembering things. I try to be about writing code so that I don’t have to remember things. In this instance, I didn’t do that, and the bear bit me.
Just a minor bite, but the fact that I need to run the program to be sure that certain things work is a bit concerning. I’ll try to “bear” down a bit on testing and other ways of being sure that everything works without a game run.
See you next time, if you can “bear” it.
P.S. I wonder … should it be called caller_id instead of modifier?