[gwt-contrib] Re: Review: JsArrays patch

2009-03-31 Thread Scott Blum
Actually, I didn't mean that it was going to pull in arbitrary amounts of code. I just literally meant that the body of that toString() is pretty big for a micro-app, like a small gadget based on GQuery. I just don't see the value of adding that code into a web mode production build. On Tue,

[gwt-contrib] Re: Review: JsArrays patch

2009-03-31 Thread Thomas Broyer
On 31 mar, 22:29, Ray Cromwell cromwell...@gmail.com wrote: Wouldn't this cause a problem when you want more than one JSO to implement Iterable? I wouldn't make the class itself implement Iterable, I'd add a helper method to return an Iterable, e.g. public class JsArrayT extends

[gwt-contrib] Re: Review: JsArrays patch

2009-03-31 Thread Freeland Abbott
The for (T x: array) ... use case is precisely what's motivating me, yes; I don't have any intention of e.g. implementing Iterator.remove(). We allow that syntax on real Java arrays, but not on JsArrays. Does returning T[] actually avoid creating an Iterator object, or does it just create an

[gwt-contrib] Re: Review: JsArrays patch

2009-03-31 Thread Ray Cromwell
Yes, My concern is, if you don't make a method that returns a T[], and instead make a JSO that implements IterableT, you can only do this once. Java treats T[] differently than IterableT with regard to the for-each loop. As usual in Java, the primitive types are treated non-orthogonally. -Ray

[gwt-contrib] Re: Review: JsArrays patch

2009-03-31 Thread Eric Ayers
On Tue, Mar 31, 2009 at 6:42 PM, Freeland Abbott fabb...@google.com wrote: The for (T x: array) ... use case is precisely what's motivating me, yes; I don't have any intention of e.g. implementing Iterator.remove(). We allow that syntax on real Java arrays, but not on JsArrays. Does

[gwt-contrib] Re: Review: JsArrays patch

2009-03-30 Thread Kelly Norton
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).

[gwt-contrib] Re: Review: JsArrays patch

2009-03-27 Thread Bruce Johnson
Let's not add this extra type JsArrayBase into the hierarchy. Why can't we just push the various methods down? We can always factor upward in the future. If we need shared implementation, we can factor that out into a package-private JsArrayImpl class. On Fri, Mar 27, 2009 at 1:28 PM, Freeland

[gwt-contrib] Re: Review: JsArrays patch

2009-03-27 Thread Kelly Norton
FWIW, in another little project I used a pattern for this that avoids implementation inheritance that I call self-delegation. Here's an example: /** Not put API, but it includes the impl for al getters and setters for all types. **/ final class JsArray extends JavaScriptObject { ... public int

[gwt-contrib] Re: Review: JsArrays patch

2009-03-27 Thread Scott Blum
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

[gwt-contrib] Re: Review: JsArrays patch

2009-03-27 Thread Freeland Abbott
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,

[gwt-contrib] Re: Review: JsArrays patch

2009-03-27 Thread Bruce Johnson
Kelly, since you have experience with this, I'd like you to be the decider (i.e. Freeland is now waiting on your LGTM). On Fri, Mar 27, 2009 at 2:16 PM, Freeland Abbott gwt.team.fabb...@gmail.com wrote: I think the argument is more for unnecessary rather than bad... although without