W3C home > Mailing lists > Public > public-webapps@w3.org > July to September 2009

Re: [File API] events vs callbacks (was: Re: New FileAPI Draft | was Re: FileAPI feedback)

From: Jonas Sicking <jonas@sicking.cc>
Date: Fri, 14 Aug 2009 15:49:58 -0700
Message-ID: <63df84f0908141549x3cd935acm3f6c94334159f36d@mail.gmail.com>
To: Garrett Smith <dhtmlkitchen@gmail.com>
Cc: Anne van Kesteren <annevk@opera.com>, arun@mozilla.com, Web Applications Working Group WG <public-webapps@w3.org>
I think at this point I would like to stop having meta-discussions
about which coding pattern is the cleanest one. I think we simply
disagree there.

In order to move forward I think we need to compare actual proposals
of APIs and see how well they solve various use cases. As far as I can
see we have three concrete proposals:

A) The proposal in the current draft [1]
B) The proposal from Olli [2]
C) The proposal from me [3]

I have so far not seen a concrete proposal from you, though what you
have described has sounded similar to [3], but with some properties
with different names (oncomplete rather than onloadend I think?)

The use cases that have been discussed so far are:

1. Reading data from the file in various formats (data-url, binary, text)
2. Progress notifications while reading the file
3. Ability to hide UI once reading is done, weather reading succeeded
or not. Do this without having to duplicate code.
4. Ability to pass File object around without risking other users of
the file interfering with your reads.

It seems to me that [1] supports 1 and 3. 2 isn't satisfied because
there are no progress notifications. It has been suggested (by me and
Arun) that this can be fixed, but no proposal has been made yet. 4
isn't satisfied because there is a risk that someone else calls
cancelReads, which aborts your reads.

[2] satisfies 1, 2 and 3. 4 isn't satisfied because if anyone calls a
getAsX function, then the events from that read will be intermingeled
with your events. Alternatively, if we don't allow parallel reads, if
someone else has started a read then you are blocked from reading
which means that 4 still isn't satisfied.

[3] satisfies 1, 2, 3 and 4, however it is the most complex API out of
any API proposed yet (not surprising as that tends to happen when you
try to satisfy all use cases :) ).

/ Jonas

[1] http://dev.w3.org/2006/webapi/FileUpload/publish/FileAPI.html
[2] http://lists.w3.org/Archives/Public/public-webapps/2009JulSep/0539.html
[3] http://lists.w3.org/Archives/Public/public-webapps/2009JulSep/0565.html

P.S. I assure you I've read your full email below, even though I
haven't responded to the individual parts.

On Fri, Aug 14, 2009 at 11:09 AM, Garrett Smith<dhtmlkitchen@gmail.com> wrote:
> On Mon, Aug 10, 2009 at 6:48 PM, Jonas Sicking<jonas@sicking.cc> wrote:
>> On Sat, Aug 8, 2009 at 9:37 AM, Garrett Smith<dhtmlkitchen@gmail.com> wrote:
>>> On Thu, Aug 6, 2009 at 11:31 AM, Jonas Sicking<jonas@sicking.cc> wrote:
>>>> On Wed, Aug 5, 2009 at 4:26 AM, Anne van Kesteren<annevk@opera.com> wrote:
>>>>> On Wed, 05 Aug 2009 10:04:28 +0200, Arun Ranganathan <arun@mozilla.com>
>>>>> wrote:
>>>>>>
>>>>>> In the case of file read APIs, simply getting the data asynchronously is
>>>>>> more convenient than using events.  There is no intrigue at work here,
>>>>>> merely disagreement.
>>>>>
>>>
>>> Who said "getting data asynchronuosly" is less convenient than "using
>>> events"? I am not sure what that really means, but I am pretty sure
>>> this is not a debate of "using events" vs. "getting data
>>> asynchronously".
>>>
>>> Arun also argued previously: "Callbacks are used so that these APIs
>>> can behave asynchronously." The callback parameter has nothing
>>> whatsoever to do with the API being asynchronous. How the callback is
>>> registered is a design decision.
>>>
>>> It was determined and agreed that reading a file should be
>>> asynchronous. We should all be on the same page now.
>>>
>>> Event callbacks are asynchronous. So, it is not a matter of arguing
>>> "synchronous events" with "asynchronous callback".
>>>
>>> Problems with the existing design were already explained in the other
>>> thread. Some of those problems were not acknowledged by the author.
>>> There is not a consensus.
>>>
>>> I would like to register a Formal Objection.
>>>
>>> The File API design does not provide for standard callback
>>> registration, but instead uses its own callback registration
>>> mechanism. This limits the flexibility of implementation code and
>>> limits extensibility of the API.
>>>
>>>>> I could imagine that for reading data you might want to have events though
>>>>> so that in the future we can introduce progress events if that is found
>>>>> necessary. E.g. if the actual file is not on the same computer as where the
>>>>> user selected it.
>>>>
>>>
>>> An API should generally be extensible.
>>>
>>> Problem: The File API couples the call to read with the callback.
>>> This violates OCP.
>>>
>>> This is easily achievable by decouple the call to "read" from the
>>> notification that the reading has completed.
>>>
>>> The way to solve this problem entails the reader implementing either:
>>>  1) DOM Events
>>>  2) DOM Event handler properties
>>
>> Skipping this part in order to get to the meat of the discussion which
>> is the actual proposal. I hope that this is ok. I assure you I read
>> all of the above.
>>
>>>> Do you have a proposal for what this would look  I'm not excited
>>>> about creating something that's significantly more complex than the
>>>> current API.
>>>>
>>>
>>> Removing the callback parameter and using a Reader was proposed in
>>> that other thread. While it was argued that it had fewer LOC in the
>>> simplest cases, it wasn't ever demonstrated as being more complicated.
>>>
>>> The current possibility is to have just a callback:-
>>>
>>> file.getAsDataURL(handleURL);
>>>
>>> - and then check the status to see if it was successful.
>>>
>>> How to implement multiple callbacks with that?
>>>
>>> Multiple callbacks can be implemented with the current draft proposal,
>>> e.g. ala "file.read( callWhenDone )", but not without a good amount of
>>> work. It would entail the program to create its own Reader that is
>>> itself an EventPublisher[1] Depending on the situation, that could be
>>> elegant or could be extra plumbing.
>>
>> I so far have not seen any use case that required the need for
>> multiple callbacks, or indeed where multiple callbacks would even be
>> useful. The use case from the previous thread seemed to be to hide UI
>> once file loading was done, no matter if the load succeeded or not.
>> The easiest way to do that seems to be:
>>
>> file.getAsBinary(myHandler);
>>
>> function myHandler(data, error) {
>>  ... hide UI ...;
>>
>>  if (error) {
>>    ... handle error ...
>>    return;
>>  }
>>
>>  ... process data ...
>> }
>>
>
>
>
> Regarding the callback function you've written, one parameter should
> be all that is necessary.
>
> - function myHandler(data, error)
> + function myHandler(ev) {
>    ...
>    if(ev.readFailure) {
>       ...
>    } else {
>        var data = ev.data;
>    }
> }
>
> I disagree that adding if-else's to the callback is better. It gives
> the callback more generic functionality.
>
> The added conditional logic increases the indentation and makes it
> harder to see what the method is doing. Testing that callback is going
> to require many tests, to cover all conditions. Now it's a toy
> example, but imagine the function gets longer (as functions tend to
> do) and becomes something like 50 LOC and has more indentations. Isn't
> it better to have a couple of clearly defined methods that follow SRP?
>
>> If we used a reader (as proposed below) together with progress events,
>> you'd get code like:
>>
>> reader = getAReader();
>> reader.onerror = function() {
>>  ... handle error ...
>> }
>> reader.onload = function() {
>>  ... process data ...
>> }
>> reader.onloadend = function() {
>>   ... hide UI ...
>> }
>> reader.read(file);
>>
>
> Only where one wanted it. If you read what I wrote:-
>
>  var reader = getAReader(); // etc.
>  reader.oncomplete = readDone;
>  reader.read( file );
>
> - there is a reader with oncomplete only. But then, if the author is
> having conditional logic inside the complete handler to determine if
> there was an error, if there was success, failure, etc. Those
> conditions might best be moved elsewhere, so that each method
> unambiguously does one thing, not looking like a sea of indentation of
> if-else's. Much easier to read and follow simple code like that and it
> will be much much easier to test and refactor because it is clean and
> decoupled.
>
> In the callback parameter example you posted and Reader example I
> posted, both could be refactored to have a readDone method as:-
>
> function readDone(ev) {
>    if(ev.isError) {
>        handleError(ev);
>    } else (
>        handleSuccess(ev);
>    }
> }
>
> (provided an "isError" property on the payload).
>
> The problem with the callback-parameter design is when the program
> using that API is designed in a way where objects are decoupled. One
> object in that program, call it UIMessage, wants to know when read is
> file.complete because it needs to handle removing the UI busy
> indicator. Another object in that program, DataTable (etc), wants to
> listen for file.success, so it can get the file content payload.
> DataTable and UIMessage know nothing about each other, only listen for
> events.
>
> * UIMessage object that wants to listen to complete from the read and
> that takes care of messaging ONLY.
>
> * a JSONParser listens for SUCCESS
>
> In a simple example, only one callback is fine.
>
> For an application that wants multiple callbacks, the strategy is either:
> Current API, callback param:
> 1) write spaghetti
> 2) write an event publishing adapter for file reads
>
> API that supports events
> 1) use multiple callbacks.
>
> Now what is interesting is the inception of an abstraction, such as
> UIMessage. In a simple case, it might be a function
> "showMessage(msg)". As the site evolves, there may be cases where the
> function is extended to handle different types of messages and
> messages that appear in different places. Now we have:-
>
> showMessage(msg, parentElementId, className);
>
> function showMessage is gets to the point where it wants to be a
> reusable object that encapsulate all the variance. The logic in the
> if-else's gets extracted and moved to methods.
>
> The proliferation of broken and badly designed garbage of the web
> (AISI) does not serve as an example of what works. It should be an
> example to learn from. A dynamic language like javascript makes it
> really easy to make an awful mess. However, the flip side of the coin
> is that a dynamic language like javascript makes it really easy to
> make flexible APIs. All the client of a Reader would want to know is:-
> 1) Can it read the file?
> 2) Can I listen for success/failure/complete?
>
>> I can't really say that this seems like a big benefit.
>
> The benefit is that I can have more than one callback if I need it. It
> is sometimes needed.
>
>>
>>> The proposed Reader solution:
>>>
>>>  var reader = getAReader(); // etc.
>>>  reader.oncomplete = readDone;
>>>  reader.read( file );
>>>
>>> This allows new features to be added, allowing forwards changes
>>> without changing existing features.  This also has a benefit of
>>> providing an Interface (Reader) that 1) reads, and 2) fires events.
>>> The reader could be any sort of reader.
>>
>> What do you mean by "The reader could be any sort of reader"?
>>
>
> Any object that has a read method and implements the events of Reader.
> The client does not need to care what sort of reader it is behind the
> scenes, so long as it is capable of
> 1) read( file )
> 2) firing callbacks
>
> A factory would be a simple way for that. That is why I had a
> "getAReader()" in the example that you read. Lets not expose a bunch
> of constructors; just a method to get a reader. I can't think of
> another type of Reader I'd want. For JSON, I could use the forthcoming
> JSON.parse. For CSV, I could probably use fileContents.join(",").
>
>>> Using a Reader decouples the type of read from the file object.
>>>
>>> The other proposed solution was to have the events on the file, not as
>>> a separate Reader.
>>>
>>> file.oncomplete = contentsRead;
>>> file.readAsDataUrl();
>>> file.readAsText();
>>>
>>> That puts the context on the file. This means the callback must check
>>> the result of the type of read. In the example above, "contentsRead"
>>> would have to know something about the data read (is it a URL or
>>> Text?) and would end up having "if else" statements that call other
>>> functions. This was provided in Jonas' example of reading file raw
>>> data and as a data-url.
>>
>> I'm strongly against this design. See my recent post at
>> http://lists.w3.org/Archives/Public/public-webapps/2009JulSep/0547.html
>> for why. Basically using the same object to both represent a file
>> resource, as well as a read-request from that resource seems very
>> prone to bugs.
>>
>
> I can see that. The same is also true for file.getAsURL( handler ). It
> puts a lot of responsibility on the File. Really, that reading
> behavior does not belong on the file. I think of the file as being a
> simple object that has a few properties (size, name, etc). It seems
> wrong to have the File object doing the reading of itself.
>
> The Reader does only reading. This means that if the read varies from
> URI or BINARY or something else (JSON, etc), then a reader suited for
> that can read it. The reader reads a file or fails. Calling read(
> aFile ) while another read of aFile is in progress should not result
> in an error and the caller should not be burdened with checking the
> aFile's state.
>
>>>> / Jonas
>>>>
>>>
>>> Regards,
>>>
>>> Garrett
>>> [1] An EventPublisher keeps track of subscribers (other callbacks),
>>> provides its own mechanism for registration), and fires events.
>>
>> / Jonas
>>
>
> Garrett
>
Received on Friday, 14 August 2009 22:50:58 GMT

This archive was generated by hypermail 2.3.1 : Tuesday, 26 March 2013 18:49:33 GMT