Few things: Overall, I'd like to be more conservative landing things in JavaScriptObject for a couple of reasons: (1) It's hard to take a mulligan with these because of their constraints (2) there is always a trivial work around to create application specific subclasses (with toll free casting).
>> From r5082: I don't think toSource is appropriate for JavaScriptObject. It only works on mozilla browsers. >> JsArray.push: As I recall, this[this.length] = value is faster than this.push(value) on all browsers. It's not a complexity change like array.pop() is, but it can be significant. (How I do wish we had continuous perf testing). >> javadoc: The javadoc for these should be written to describe what the function does. "Direct mapping to underlying sort" is a good implementation note, but we should actually way what it does and not rely on developers having an understanding of the JavaScript equivalent. >> sort(JavaScriptObject): I'd like to avoid this one if we can. It just opens up larger questions about the right way to do this. We don't currently have a convention for representing JavaScript functions in Java. Someone should probably have a good story there before we add something like this to JavaScriptObject. /kel On Fri, Mar 27, 2009 at 2:15 PM, Freeland Abbott <fabb...@google.com> wrote: > I think the argument is more for "unnecessary" rather than "bad"... > although without JsArrayBase (we can make it package-protected, and call it > JsArrayImpl if anyone cares), we duplicate the JSNI implementation for a > couple trivial methods. I thought refactoring them into one place was nice, > but trivial enough that I'm not fighting over it. Revised patch attached; I > can go either way on this. > > > > > > On Fri, Mar 27, 2009 at 2:06 PM, Scott Blum <sco...@google.com> wrote: > >> I'm going to punt this review to Bruce & Kelly, 'cause I have no idea why >> having JsArrayBase would be bad. :) >> >> On Fri, Mar 27, 2009 at 1:28 PM, Freeland Abbott < >> gwt.team.fabb...@gmail.com> wrote: >> >>> Scott, we already talked about this, but here's the patch for public >>> review. >>> >>> The basic goal is to surface the native length, sort, push, and shift >>> operators for JsArrays... I know you mentioned that IE6's push may be slower >>> than indexed extension, and thus a candidate for deferred binding, but I >>> wanted to get a basic implementation in first. >>> >>> There should be only checkstyle changes from what we discussed (though >>> that obviously doesn't help the rest GWTC). I also added some checkstyle >>> fixes to JavaScriptObject, introduced by my c5082. >>> >> >> > -- If you received this communication by mistake, you are entitled to one free ice cream cone on me. Simply print out this email including all relevant SMTP headers and present them at my desk to claim your creamy treat. We'll have a laugh at my emailing incompetence, and play a game of ping pong. (offer may not be valid in all States). --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
