RawCcashAsyncCallback fires LoadingStopEvents on success or failure. However, it is the manually defined RPCProxy classes that fire the LoadingStartEvents; in fact that is their primary, if not only, purpose.
Paul's reason for this, instead of firing a LoadingStartEvent in the RawCcashAsyncCallback constructor was in case a CcashAsyncCallback were created that did not need to be timed. In fact, there are several RawCcashAsyncCallback events that are not timed.
Ccash.java defines listeners for LoadingStartEvent and LoadingStopEvent. On start events it adds the event's stored AsyncCallback to the WidgetFlasher. On stop events it removes the event's stored AsyncCallback from the WidgetFlasher.
Proposed
I propose to rework the CcashAsyncCallback hierarchy and localize the firing of loading start/stop events.
Make CcashAsyncCallback the base class that defines common functionality, perhaps the current onFailure implementation. It will not fire any loading events.
Make a TimingCcashAsyncCallback that will fire a loading start event in the constructor and loading stop events on success or failure. Then, where timing is desired, anonymous TimingCcashAsyncCallback objects should be defined instead of CcashAsyncCallback objects.
In fact, the TimingCcashAsyncCallback could actually just track the start and stop times and log a tag directly to the timeline and a separate async callback class could be defined to fire loading start/stop events which would control the display of the loading indicator. In this case perhaps the decorator or strategy pattern should be applied.
Using the decorator/strategy pattern would also require a factory/builder pattern to simplify the definition of anonymous/in-line async callback objects to have the correct decorator chain.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Currently
RawCcashAsyncCallback fires LoadingStopEvents on success or failure. However, it is the manually defined RPCProxy classes that fire the LoadingStartEvents; in fact that is their primary, if not only, purpose.
Paul's reason for this, instead of firing a LoadingStartEvent in the RawCcashAsyncCallback constructor was in case a CcashAsyncCallback were created that did not need to be timed. In fact, there are several RawCcashAsyncCallback events that are not timed.
Ccash.java defines listeners for LoadingStartEvent and LoadingStopEvent. On start events it adds the event's stored AsyncCallback to the WidgetFlasher. On stop events it removes the event's stored AsyncCallback from the WidgetFlasher.
Proposed
I propose to rework the CcashAsyncCallback hierarchy and localize the firing of loading start/stop events.
Make CcashAsyncCallback the base class that defines common functionality, perhaps the current onFailure implementation. It will not fire any loading events.
Make a TimingCcashAsyncCallback that will fire a loading start event in the constructor and loading stop events on success or failure. Then, where timing is desired, anonymous TimingCcashAsyncCallback objects should be defined instead of CcashAsyncCallback objects.
In fact, the TimingCcashAsyncCallback could actually just track the start and stop times and log a tag directly to the timeline and a separate async callback class could be defined to fire loading start/stop events which would control the display of the loading indicator. In this case perhaps the decorator or strategy pattern should be applied.
Using the decorator/strategy pattern would also require a factory/builder pattern to simplify the definition of anonymous/in-line async callback objects to have the correct decorator chain.
Commit 272 (CCF). Ticket [Trac] #425. Added comment to the WidgetFlasher.remove on the probable cause for exceptions when removing objects.