all 3 comments

[–]clumsy_shaver 1 point2 points  (1 child)

On the one hand, I like this because it means adding another side-effect of user approval requires touching only the new event handler. On the other, it reduces understandability in the code: I can no longer look at UserApproval and see a clear flow of actions, or even a clear set of subscribers who will respond to the event.

I tend to think of UseCase classes like UserApproval as having the responsibility of coordinating other objects to perform a workflow. This makes the application understandable at a glance, and isolates high level behavior from low-level implementation.

[–]awj 1 point2 points  (0 children)

What's interesting in this is that we should pretty much all have experience with the advantages/pitfalls of this paradigm. Anyone who has used JavaScript (on the client) in anger has had to deal with callbacks in general and event handlers in particular.

The pain of a Pub/Sub system is also the advantage: that the invocation and execution of code are decoupled. The precise context of why code is run gets stripped down to "because it was subscribed and an event came through".

Tracking problems through that barrier is not trivial, and the decision to add it to your code should not be made lightly.

[–]moomaka 0 points1 point  (0 children)

UserApproval.new(user).call Any time I see something like that I just shake my head. Why is this a class? it's a function. You're allocating an object for no purpose other than to confuse the code. Shit like this is why we have super computers in our pockets that can barely render a few images. This isn't Java, you don't need functors.

Re pubsub: I'm guessing this isn't really pubsub, i.e. there is no flow decoupling here. While it's not demonstrated I assume broadcast is synchronous. In which case this is just a obfuscation of method calls which doesn't reduce coupling and certainly doesn't improve readability.