se-edu/addressbook-level4

UI components: apply delegation pattern for inter-VC communication

Zhiyuan-Amos opened this issue · 6 comments

Edit: This is part of my CS2103R's work which has been approved by prof, and we would like to hear comments from other developers on how we can improve this.


MainWindow is a container that contains the other View-Controllers(VCs) such as CommandBox, BrowserPanel and PersonListPanel. Whenever 2 child VCs of MainWindow need to communicate to one another, they do so by posting an event. For these VCs to catch events, they must first register themselves as an event handler and to implement a method to handle the event.

Raising an event for inter-View-Controllers communication is unnecessary as:

  1. All these communications only occur between one View-Controller and another
    View-Controller i.e. there's only 1 listener for each of these events.
  2. Events are used for broadcasting across different components and classes that are
    unrelated to each other. For example, whenever the Model is updated, it will post an AddressBookChangedEvent to notify StatusBarFooter (a View- Controller) and the Storage Component. However, these View-Controllers are children of MainWindow and are therefore closely related. As such, this doesn’t quite fit the use case of events[1].

We can remove the need to use events for communication between View-Controllers by teaching MainWindow to follow the Delegation Pattern[2]. For example, whenever a new card in PersonListPanel is selected, instead of posting an Event to be caught by the BrowserPanel, it can call delegate.newCardSelected(newValue) and MainWindow will then call the corresponding method in BrowserPanel. Example of the UML diagram:

image

By using Delegate Pattern instead of events, we achieve:

  1. Code traceability, therefore easier to debug as well.
  2. More compile-time checks. If we forget to teach a VC to catch a corresponding Event, there is no compile-time check to inform us of this error. However, if we forget to teach MainWindow to implement the interface and the methods in the interface, there will be a compile-time error.
  3. VCs such as BrowserPanel no longer needs to register itself as an event handler.
[1]: https://sandofsky.com/blog/delegates-vs-observers.html
[2]: http://www.andrewcbancroft.com/2015/03/26/what-is-delegation-a-swift-developers-guide/

Does this mean newCardSelected will be a public method on MainWindow?

Might be better for MainWindow to pass in a callback to PersonListPanel using lambdas, similar to what JavaFX does.

@pyokagan yup, do you have a link for me to reference and better understand? :)

@Zhiyuan-Amos Are you still planning on doing this? If so, what's the time frame?

@pyokagan I'm not too sure; been really busy. I'll get back by recess week (5-6weeks time)?

Nevermind, I realized afterwards that this won't block AB3, since it's UI-specific.