From: Stefan Weiss on
On 30/06/10 16:38, Scott Sauyet wrote:
> Stefan Weiss wrote:
>> At the other end of the spectrum, you could go in a more object oriented
>> direction and create Job objects which would know how they should be
>> transported, and which an Aggregator could then run and manage. This
>> could also help to eliminate the "stringiness" of your job identifiers.
>>
>> PS: I just saw Asen's reply. It looks like his RequestWrapper objects
>> are similar to what I was trying to say in my last paragraph.
>
> Yes, I hadn't really considered going that far, but it might really be
> useful. I'm not quite sure of the API, though. Are you thinking
> something like this?:
>
> var agg = new Aggregator(myHandler);
> var job1 = new Job(function() {
> getJsonAjax({
> url: "red/",
> success: function(data, status) {job1.stop(data);},
> failure: function(status, err) {job1.error(status);}
> });
> });
> agg.add(job1);

That could work. I'm not a big fan of using the job1 variable the way
you did; I think an object should always know "itself", instead of
depending on an external variable. In this example, it's perfectly clear
that job1 will point to the correct object, but in theory, the two are
independent - job1 could (theoretically) be redefined before the success
or failure callbacks are called. Probably not a big deal, and you could
get around it by using an appropriate scope, but it still makes me a
little uncomfortable.

Here are some more random thoughts and musings; they are mostly based on
my personal preferences, well into the "overkill" area, and not intended
as any kind of concrete advice or criticism :)

As I said, I'm not a great code reuser (in the classical sense). Given
that, I would still like to integrate the Ajax functionality into one of
the objects - in this case, the Job object. I think if you find yourself
duplicating code a lot, as you would do with the whole getJsonAjax
thing, that's an indication that the API could be simplified. It seems
to me that the majority of requests will be similar enough to make this
work, and you could still cater for different request types by creating
more than one Job "class". There could be an AjaxJob, a WebWorkerJob,
etc, and they could all be managed by the same Aggregator and handler as
long as they present the same public interface.

I wouldn't worry much about making one of the Job "classes" depend on an
application specific transport mode. The Aggregator could stay generic,
same as the handler and the Job "interface". Only the current
application would want to create a new specialized Job type.

Or, to rephrase it in a way that some of my previous employers would
have loved...

Create an interface IJob, a JobFactory class, and then several
specialized *Job classes implementing some common methods like send(),
abort(), getData() etc. Then you'll need a JobEvent subsystem for
message passing between the Aggregator, Jobs, and JobHandlers - we frown
on circular dependencies. You may also need several different JobHandler
classes, depending on which event you want to handle. You *might* get by
with a generic JobHander, but that's not nearly enterprisey enough. I'd
go with an IJobHandler interface, an abstract JobHandler class, and then
the obvious JobErrorHandler, JobCompleteHandler, [..] classes. When
you're finished with this System (capital S! ETA: 2012), remember to
request at least two full-time QA testers, and no less than 17
developers to maintain the System. Oh, and some more managers. More
managers never hurt.
I forgot to mention all the JobDecorators, JobAdapters, JobMediators and
JobFacades. My bad.

Sometimes I'm very glad and thankful that JS enables us to get on with
our work with a minimum of fuss, even if it's less enterprisey than J
without the S.


--
stefan
From: Thomas 'PointedEars' Lahn on
Scott Sauyet wrote:

> var agg = new Aggregator();
>
> var myHandler = function(data, statuses, errors) {
> // statuses = {"blue": "completed", "red": "completed",
> // "yellow": "completed"}
> // errors = {} //
> // data = {"red": { /* ... */}, "blue": { /* ... */ },
> // "yellow": { /* ... */ }}
>
> // do something with data.blue, data.red, data.yellow
> }
>
> agg.onComplete(myHandler);

The `myHandler' variable does not appear to serve a useful purpose here.
Maybe it is only because of the example.

Make it

agg.oncomplete = myHandler;

if viable for your application design. Saves one call and is more
intuitive.

> // Or var agg = new Aggregator(myHandler);
>
> agg.start("blue");
> agg.start("red");
> agg.start("yellow");
>
> getJsonAjax({

Rename getJsonAjax() and make it a method of `agg' inherited from
`Aggregator.prototype' that does not send(), and you will not have to worry
about agg.start(). Then agg.send() at last.

> [...]
> The user associates each asynchronous process with a String (above
> "red", "blue", and "yellow"). When each process completes, the
> relevant returned data is supplied with the complete() call.
>
> The completion handlers accept three parameters. The first represents
> the actual results of the various asynchronous calls, the second, the
> status flags of each call ("started", "completed", "error", possibly
> "aborted"), and the third, any error messages generated by these
> calls. Any actual combination of the results is left to the caller,
> presumably in the completion handlers. You can pass such handlers to
> the onComplete() method or to the constructor.
>
> I do have an implementation of this, but at the moment, I'm mostly
> concerned with whether the API makes sense. Is there a cleaner way to
> do this? Is this general enough? Are the String identifiers for the
> calls robust enough?

Use either "constants" or object references instead. As for the latter, let
the caller deal with names when and if they need them (per variables or
properties).

> Should the constructor also accept the identifiers?

I do not think it a bad idea if the callback(s) could optionally be supplied
with the constructor call. I have done so in JSX:httprequest.js myself.
BTW, thanks for this "aggregator" idea.

> What suggestions do you have?

HTH


PointedEars
--
Prototype.js was written by people who don't know javascript for people
who don't know javascript. People who don't know javascript are not
the best source of advice on designing systems that use javascript.
-- Richard Cornford, cljs, <f806at$ail$1$8300dec7(a)news.demon.co.uk>
From: Scott Sauyet on
Stefan Weiss wrote:
> On 30/06/10 16:38, Scott Sauyet wrote:
>> I'm not quite sure of the API, though.  Are you thinking
>> something like this?:
>
>>     var agg = new Aggregator(myHandler);
>>     var job1 = new Job(function() {
>>       getJsonAjax({
>>         url: "red/",
>>         success: function(data, status) {job1.stop(data);},
>>         failure: function(status, err) {job1.error(status);}
>>       });
>>     });
>>     agg.add(job1);
>
> That could work. I'm not a big fan of using the job1 variable the way
> you did; I think an object should always know "itself", instead of
> depending on an external variable. In this example, it's perfectly clear
> that job1 will point to the correct object, but in theory, the two are
> independent - job1 could (theoretically) be redefined before the success
> or failure callbacks are called. Probably not a big deal, and you could
> get around it by using an appropriate scope, but it still makes me a
> little uncomfortable.

Yes, I'm uncomfortable with it too. But I'm still not quite seeing a
way around that unless the AJAX function is itself a method of the Job
object. Perhaps that's not such a bad idea, though, and relates to
what you say below. I will think about it.

> Here are some more random thoughts and musings; they are mostly based on
> my personal preferences, well into the "overkill" area, and not intended
> as any kind of concrete advice or criticism :)

In any case, they're much appreciated.


> As I said, I'm not a great code reuser (in the classical sense). Given
> that, I would still like to integrate the Ajax functionality into one of
> the objects - in this case, the Job object. I think if you find yourself
> duplicating code a lot, as you would do with the whole getJsonAjax
> thing, that's an indication that the API could be simplified. It seems
> to me that the majority of requests will be similar enough to make this
> work, and you could still cater for different request types by creating
> more than one Job "class". There could be an AjaxJob, a WebWorkerJob,
> etc, and they could all be managed by the same Aggregator and handler as
> long as they present the same public interface.

I think that's right. If I work up another iteration of this (I've
already had to put the first iteration into production; amazing how
quickly pie-in-the-sky can turn into we-need-this-by-yesterday!) I
will try it that way. A Job object can be started or aborted, it has
a status, can contain a result, and it fires completed and error
events, which the Aggregator will listen to. This makes more sense
than what I have now.


> I wouldn't worry much about making one of the Job "classes" depend on an
> application specific transport mode. The Aggregator could stay generic,
> same as the handler and the Job "interface". Only the current
> application would want to create a new specialized Job type.

Yes, this is definitely an improvement.

> Or, to rephrase it in a way that some of my previous employers would
> have loved...

I'm not going to quote that here. It would be funny if it weren't so
scarily familiar!


> Sometimes I'm very glad and thankful that JS enables us to get on with
> our work with a minimum of fuss, even if it's less enterprisey than J
> without the S.

Isn't it funny how the ability to just get things done is downplayed
in enterprise systems?

Thank you again. Your responses have been very helpful.

--
Scott
From: Scott Sauyet on
Thomas 'PointedEars' Lahn wrote:
> Scott Sauyet wrote:
>>     var agg = new Aggregator();
>
>>     var myHandler = function(data, statuses, errors) {
>> // [ ... ]
>>       // do something with data.blue, data.red, data.yellow
>>     }
>
>>     agg.onComplete(myHandler);
>
> The `myHandler' variable does not appear to serve a useful purpose here.  
> Maybe it is only because of the example.

In practice, I would almost certainly pass an anonymous function to
the constructor or to the event registration method. I was just
trying to make things more explicit for the example.

> Make it
>
>   agg.oncomplete = myHandler;
>
> if viable for your application design.  Saves one call and is more
> intuitive.

I think it's just old habit to allow for multiple listeners. I can't
think of any circumstances where I would use more than one here, so it
might be best to drop the event registration and simply pass it in the
constructor.


>>     // Or var agg = new Aggregator(myHandler);
>
>>     agg.start("blue");
>>     agg.start("red");
>>     agg.start("yellow");
>
>>     getJsonAjax({
>
> Rename getJsonAjax() and make it a method of `agg' inherited from
> `Aggregator.prototype' that does not send(), and you will not have to worry
> about agg.start().  Then agg.send() at last.

That could work. But I think I like Stefan's suggestion of Job
objects being managed by the aggregator. I don't have it all worked
out yet, but something like:

var agg = new Aggregator([
new AjaxJob("red/"),
new AjaxJob("blue/"),
new AjaxJob("yellow/"),
new OtherJob("whatever", "parameters")
], myCallback);

agg.start(); // maybe not necessary.


>> I do have an implementation of this, but at the moment, I'm mostly
>> concerned with whether the API makes sense.  Is there a cleaner way to
>> do this?  Is this general enough?  Are the String identifiers for the
>> calls robust enough?
>
> Use either "constants" or object references instead.  As for the latter, let
> the caller deal with names when and if they need them (per variables or
> properties).

I've been assuming that the callback function does the actual
aggregation (which perhaps means that the main class is poorly
named.) For my current needs, the results of the individual calls all
have the same structure, and I could use some generic aggregation, but
I would rather not assume it will stay that way, so I need some means
to distinguish the various results from one another, but using
Stefan's technique, this could be handled simply by using the job
objects.


>> Should the constructor also accept the identifiers?
>
> I do not think it a bad idea if the callback(s) could optionally be supplied
> with the constructor call.  I have done so in JSX:httprequest.js myself..  

Yes, that was always my intent. But in the above, I can also supply
the initial set of Job objects to the Aggregator.


> BTW, thanks for this "aggregator" idea.

You're welcome. I can't take credit for the name. I had the idea,
and could not come up with a name for it. It was driving me a little
crazy, too. A coworker came up with "Aggregator" half-way through my
explanation.


>> What suggestions do you have?
>
> HTH

Very much so. Thank you very much.

--
Scott
From: Asen Bozhilov on
Scott Sauyet wrote:

> It probably is enough for my current needs, too.  But there is a real
> possibility that I will need some additional features I didn't mention
> in my initial message, but which did prompt the API I used.  First of
> all, sometimes the asynchronous process I'm waiting for might be user
> input rather than an AJAX call.  Second, some calls might lead me to
> make additional ones, and I want my wrap-up function to run only after
> all the results are in.  I might still be able to get away without
> listing the calls, though, and only counting, but I don't think I
> could use the push-push-push-start system, but rather one in which
> each process is started and the aggregator checks after each one
> completes whether there are still any processes still running.

Probably by these requirements the follow is enough.

function Aggregator() {
var that = this;
this.data = [];
this._count = 0;

this._handler = function (request, response) {
that.data.push({response : response, request : request});
request.success = null;
that._count--;
if (that._count == 0) {
this.onComplete();
}
};
}

Aggregator.prototype = {
onComplete : function () {},
add : function (request) {
this._count++;
request.success = this._handler;
}
};

var agg = new Aggregator();

agg.onComplete = function () {
/**
* Get the data of the aggregated requests
*/
this.data;
};

var req1 = new RequestWrapper(),
req2 = new RequestWrapper(),
req3 = new RequestWrapper();

agg.add(req1);
agg.add(req2);

req1.send();
req2.send();

agg.add(req3);
req3.send();

The interesting point for me is how do you want to associate response
data with the requests? And in which order? At my current API they are
in order of response not in order of adding in `Aggregator'. With
your approach with identifiers and mapping in native object you would
have one problem. You cannot support any order. If you use `for-in'
for iterating over responses you cannot know in which order they will
be enumerated, because that is implementation depended.

You should reply on these things which I wrote, and probably you would
get more clever ideas for your API. The general problem is the order
of response which you expected.