Ignacio Soler Garcia
Ignacio Soler Garcia

Reputation: 21855

SOLID & EventAggregator

I'm trying to follow SOLID as much as I can and I'm facing the following problem while working in a Prism application:

I have a class that handles all the events the module receives from other modules. As the class has two responsibilities (registering the handlers and handling the events) I decided to split the class in two which leads me to make event handlers public (which seems weird anyway).

What do you prefer in this situation, to have two responsibilities in one class or to have event handlers public (or anything else that I'm missing)?

Regards.

EDIT:

public class Handler
{
    public void MethodHandlingAggregatedEvent()
    {
    }
}

public class Register
{
    .... 

    public void RegisterHandler()
    {
        this.eventAggregator.GetEvent<XXX>().Subscribe(this.handler.MethodHandlingAggregatedEvent);
    }
}

Upvotes: 3

Views: 236

Answers (2)

Dzianis Yafimau
Dzianis Yafimau

Reputation: 2016

I think your design is mostly ok.

  1. Your class Register is responsible for event-handler mapping
  2. Each handler is responsible for handle concrete events

I would recommend to rename method RegisterHandler() to RegisterHandlers() to reflect it's behavior. Also make all classes implementing interfaces. So you can mock them in unit test like that:

_registerMock.Verify(x => x.RegisterHandlers(), Times.Once);

_eventAggregator.Invoke(SomeEvent);
_someHandler.Verify(x => x.MethodHandlingAggregatedEvent(), Times.Once);

Upvotes: 0

Bas
Bas

Reputation: 27085

To explain my answer better, consider the following example:

  • A Prism application contains ModuleX and ModuleY
  • EventA and EventB are defined somewhere where both modules can access them.
  • ModuleX contains event handlers for both events.
  • ModuleY fires these events.

Also consider the following aspects of SOLID (from Wikipedia)

Single responsibility principle: a class should have only a single responsibility (i.e. only one potential change in the software's specification should be able to affect the specification of the class)

Interface segregation principle: “many client-specific interfaces are better than one general-purpose interface.”[4]

Requirements for the functionality you describe when mapped to this example would be:

  1. Register Handler for event A
  2. Register Handler for event B
  3. Handle Event A
  4. Handle Event B

If you have a single class that handles both A and B you would be breaking the Single responsibility principle. If you register an event handler with a handler defined in the same class you would be also breaking the single responsibility principle.

So what now? Splitting up everything would yield four classes, since these apparently are all different responsibilities. According to Wikipedia the SOLID principle exists to simplifiy extending and modifying existing code; therefore it should be there to make your life easier, not harder.

If we create four classes we are forced to open up the event handler code to a scope outside of the class it was created in. In my humble opinion it would be a better choice to keep the event handling code as well as the registration in the smallest scope possible:

class MyHandler
{
    public MyHandler(IEventAggregator eventAggregator)
    {
        eventAggregator.GetEvent<EventA>().Register(HandleEventA);
    }

    private void HandleEventA(EventArgs args)
    {

    }
}

MyHandler is now responsible for handling EventA. Even though it is also registering the handler; one might still consider this as being part of the event handler.

Upvotes: 3

Related Questions