[Review request] Image loader API

[Lubomir Nerad]

Hi Richard,

On 1/30/2013 12:07 AM, Richard Bair wrote:
>> So what if we leave these event classes unrelated and keep the ImageLoader as originally proposed (without extending Worker), but add the following two methods into it:
>>
>> Worker getLoadingService();
>> Worker getLoadingTask(Image image);
>>
>> The first one will allow to get the Worker which allows to observe the loading process as whole and the second one to observe loading of a single image.
>>
>> Examples:
>>
>> 1) generic dialog with total progress
>> ImageLoader imageLoader = new ImageLoader();
>> image1 = imageLoader.loadAsync(...);
>> image2 = imageLoader.loadAsync(...);
>> image3 = imageLoader.loadAsync(...);
>> image4 = imageLoader.loadAsync(...);
>> totalProgressDialog.track(imageLoader.getLoadingService());
>>
>> 2) generic dialog with detailed progress
>> ImageLoader imageLoader = new ImageLoader();
>> image1 = imageLoader.loadAsync(...);
>> image2 = imageLoader.loadAsync(...);
>> image3 = imageLoader.loadAsync(...);
>> image4 = imageLoader.loadAsync(...);
>> detailedProgressDialog.track(
>>         imageLoader.getLoadingTask(image1),
>>         imageLoader.getLoadingTask(image2),
>>         imageLoader.getLoadingTask(image3),
>>         imageLoader.getLoadingTask(image4));
>>
>> The advantages of this solution is that it will keep ImageLoader simple, while it will allow generic progress tracking (total or detailed). We can also return some Worker subclass which would allow to register WorkerStateEvent handlers or support advanced user cases (for example define Executor for loading service).
>>
>> What do you think?
> I'm not sure I see an advantage to moving the loading service off the image loader itself, but I agree it sounds like a workable solution. I spent time last week on ScheduledService (http://javafx-jira.kenai.com/browse/RT-18702) and filed an issue for ConcurrentService (http://javafx-jira.kenai.com/browse/RT-27952) which is more like what ImageLoader could extend from (vs. the other Services). To me it is an open question as to whether it is better for ImageLoader to extend from Object but *have* a service, or whether it ought to *be* a Service (of the ConcurrentService variety, which would extend from a new ServiceBase base class rather than from Service directly the way ScheduledService does).
>
> Richard

I think it depends on how the final ServiceBase class will look like. 
For example having ImageLoader inherit on... event handler properties 
from ServiceBase, can bring confusion into ImageLoader set of event 
handling properties.

What I mean is that if I have both:

public final void setOnLoadingSucceeded(EventHandler<? super 
ImageLoadingEvent> value);
public final void setOnSucceeded(EventHandler<WorkerStateEvent> value);

in a single interface, it won't be that straightforward to know which 
one to select. While if I'll need to get a reference to the loading 
service first, it should be more clear that its on handlers will provide 
status for the whole service rather than its individual tasks.

Lubo