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:
- 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. - Events are used for broadcasting across different components and classes that are
unrelated to each other. For example, whenever theModel
is updated, it will post anAddressBookChangedEvent
to notifyStatusBarFooter
(a View- Controller) and theStorage
Component. However, these View-Controllers are children ofMainWindow
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:
By using Delegate Pattern instead of events, we achieve:
- Code traceability, therefore easier to debug as well.
- 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. - 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? :)
Something like 0bb6884#diff-88f06211563ad55170a8a9c11829abf0R120
@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.