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

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 18:10:12 UTC