> > 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. >>> >> >> >
--~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
Index: user/src/com/google/gwt/core/client/JsArrayString.java =================================================================== --- user/src/com/google/gwt/core/client/JsArrayString.java (revision 5095) +++ user/src/com/google/gwt/core/client/JsArrayString.java (working copy) @@ -1,12 +1,12 @@ /* * Copyright 2008 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -17,10 +17,10 @@ /** * A simple wrapper around a homogeneous native array of string values. - * + * * This class may not be directly instantiated, and can only be returned from a * native method. For example, - * + * * <code> * native JsArrayString getNativeArray() /*-{ * return ['foo', 'bar', 'baz']; @@ -34,7 +34,7 @@ /** * Gets the value at a given index. - * + * * @param index the index to be retrieved * @return the value at the given index, or <code>null</code> if none exists */ @@ -44,7 +44,7 @@ /** * Gets the length of the array. - * + * * @return the array length */ public final native int length() /*-{ @@ -52,15 +52,38 @@ }-*/; /** + * call underlying push method. + */ + public final native void push(String value) /*-{ + this.push(value); + }-*/; + + /** * Sets the value value at a given index. - * + * * If the index is out of bounds, the value will still be set. The array's * length will be updated to encompass the bounds implied by the added value. - * + * * @param index the index to be set * @param value the value to be stored */ public final native void set(int index, String value) /*-{ this[index] = value; }-*/; + + /** + * Shifts the first value off the array. + * @return the shifted value + */ + public final native String shift() /*-{ + return this.shift(); + }-*/; + + /** + * direct mapping to underlying sort method. + */ + public final native void sort(JavaScriptObject sortFunc) /*-{ + this.sort(sortFunc); + }-*/; + } Index: user/src/com/google/gwt/core/client/JsArrayBoolean.java =================================================================== --- user/src/com/google/gwt/core/client/JsArrayBoolean.java (revision 5095) +++ user/src/com/google/gwt/core/client/JsArrayBoolean.java (working copy) @@ -1,12 +1,12 @@ /* * Copyright 2008 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -17,10 +17,10 @@ /** * A simple wrapper around a homogeneous native array of boolean values. - * + * * This class may not be directly instantiated, and can only be returned from a * native method. For example, - * + * * <code> * native JsArrayBoolean getNativeArray() /*-{ * return [true, false, true]; @@ -34,11 +34,11 @@ /** * Gets the value at a given index. - * + * * If an undefined or non-boolean value exists at the given index, a * type-conversion error will occur in hosted mode and unpredictable behavior * may occur in web mode. - * + * * @param index the index to be retrieved * @return the value at the given index */ @@ -48,7 +48,7 @@ /** * Gets the length of the array. - * + * * @return the array length */ public final native int length() /*-{ @@ -56,15 +56,38 @@ }-*/; /** + * call underlying push method. + */ + public final native void push(boolean value) /*-{ + this.push(value); + }-*/; + + /** * Sets the value value at a given index. - * + * * If the index is out of bounds, the value will still be set. The array's * length will be updated to encompass the bounds implied by the added value. - * + * * @param index the index to be set * @param value the value to be stored */ public final native void set(int index, boolean value) /*-{ this[index] = value; }-*/; + + /** + * Shifts the first value off the array. + * @return the shifted value + */ + public final native boolean shift() /*-{ + return this.shift(); + }-*/; + + /** + * direct mapping to underlying sort method. + */ + public final native void sort(JavaScriptObject sortFunc) /*-{ + this.sort(sortFunc); + }-*/; + } Index: user/src/com/google/gwt/core/client/JsArray.java =================================================================== --- user/src/com/google/gwt/core/client/JsArray.java (revision 5095) +++ user/src/com/google/gwt/core/client/JsArray.java (working copy) @@ -1,12 +1,12 @@ /* * Copyright 2008 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -18,10 +18,10 @@ /** * A simple wrapper around a homogeneous native array of * {...@link JavaScriptObject} values. - * + * * This class may not be directly instantiated, and can only be returned from a * native method. For example, - * + * * <code> * native JsArray<JavaScriptObject> getNativeArray() /*-{ * return [ @@ -31,7 +31,7 @@ * ]; * }-* /; * </code> - * + * * @param <T> the concrete type of object contained in this array */ public class JsArray<T extends JavaScriptObject> extends JavaScriptObject { @@ -41,7 +41,7 @@ /** * Gets the object at a given index. - * + * * @param index the index to be retrieved * @return the object at the given index, or <code>null</code> if none * exists @@ -52,7 +52,7 @@ /** * Gets the length of the array. - * + * * @return the array length */ public final native int length() /*-{ @@ -60,15 +60,38 @@ }-*/; /** + * call underlying push method. + */ + public final native void push(T value) /*-{ + this.push(value); + }-*/; + + /** * Sets the object value at a given index. - * + * * If the index is out of bounds, the value will still be set. The array's * length will be updated to encompass the bounds implied by the added object. - * + * * @param index the index to be set * @param value the object to be stored */ public final native void set(int index, T value) /*-{ this[index] = value; }-*/; + + /** + * Shifts the first value off the array. + * @return the shifted value + */ + public final native T shift() /*-{ + return this.shift(); + }-*/; + + /** + * direct mapping to underlying sort method. + */ + public final native void sort(JavaScriptObject sortFunc) /*-{ + this.sort(sortFunc); + }-*/; + } Index: user/src/com/google/gwt/core/client/JsArrayNumber.java =================================================================== --- user/src/com/google/gwt/core/client/JsArrayNumber.java (revision 5095) +++ user/src/com/google/gwt/core/client/JsArrayNumber.java (working copy) @@ -1,12 +1,12 @@ /* * Copyright 2008 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -17,13 +17,13 @@ /** * A simple wrapper around a homogeneous native array of numeric values. - * + * * All native JavaScript numeric values are implicitly double-precision, so only * double values may be set and retrieved. - * + * * This class may not be directly instantiated, and can only be returned from a * native method. For example, - * + * * <code> * native JsArrayNumber getNativeArray() /*-{ * return [1.1, 2.2, 3.3]; @@ -37,11 +37,11 @@ /** * Gets the value at a given index. - * + * * If an undefined or non-numeric value exists at the given index, a * type-conversion error will occur in hosted mode and unpredictable behavior * may occur in web mode. - * + * * @param index the index to be retrieved * @return the value at the given index */ @@ -51,23 +51,47 @@ /** * Gets the length of the array. - * + * * @return the array length */ public final native int length() /*-{ return this.length; }-*/; + /** + * call underlying push method. + */ + public final native void push(double value) /*-{ + this.push(value); + }-*/; + + /** * Sets the value value at a given index. - * + * * If the index is out of bounds, the value will still be set. The array's * length will be updated to encompass the bounds implied by the added value. - * + * * @param index the index to be set * @param value the value to be stored */ public final native void set(int index, double value) /*-{ this[index] = value; }-*/; + + /** + * Shifts the first value off the array. + * @return the shifted value + */ + public final native double shift() /*-{ + return this.shift(); + }-*/; + + /** + * direct mapping to underlying sort method. + */ + public final native void sort(JavaScriptObject sortFunc) /*-{ + this.sort(sortFunc); + }-*/; + } Index: user/src/com/google/gwt/core/client/JsArrayInteger.java =================================================================== --- user/src/com/google/gwt/core/client/JsArrayInteger.java (revision 5095) +++ user/src/com/google/gwt/core/client/JsArrayInteger.java (working copy) @@ -1,12 +1,12 @@ /* * Copyright 2008 Google Inc. - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -17,10 +17,10 @@ /** * A simple wrapper around a homogeneous native array of integer values. - * + * * This class may not be directly instantiated, and can only be returned from a * native method. For example, - * + * * <code> * native JsArrayInteger getNativeArray() /*-{ * return [1, 2, 3]; @@ -34,12 +34,12 @@ /** * Gets the value at a given index. - * + * * If no value exists at the given index, a type-conversion error will occur * in hosted mode and unpredictable behavior may occur in web mode. If the * numeric value returned is non-integral, it will cause a warning in hosted * mode, and may affect the results of mathematical expressions. - * + * * @param index the index to be retrieved * @return the value at the given index */ @@ -49,7 +49,7 @@ /** * Gets the length of the array. - * + * * @return the array length */ public final native int length() /*-{ @@ -57,15 +57,38 @@ }-*/; /** + * call underlying push method. + */ + public final native void push(int value) /*-{ + this.push(value); + }-*/; + + /** * Sets the value value at a given index. - * + * * If the index is out of bounds, the value will still be set. The array's * length will be updated to encompass the bounds implied by the added value. - * + * * @param index the index to be set * @param value the value to be stored */ public final native void set(int index, int value) /*-{ this[index] = value; }-*/; + + /** + * Shifts the first value off the array. + * @return the shifted value + */ + public final native int shift() /*-{ + return this.shift(); + }-*/; + + /** + * direct mapping to underlying sort method. + */ + public final native void sort(JavaScriptObject sortFunc) /*-{ + this.sort(sortFunc); + }-*/; + } Index: user/src/com/google/gwt/core/client/JavaScriptObject.java =================================================================== --- user/src/com/google/gwt/core/client/JavaScriptObject.java (revision 5095) +++ user/src/com/google/gwt/core/client/JavaScriptObject.java (working copy) @@ -28,13 +28,6 @@ public class JavaScriptObject { /** - * Call the toSource() on the JSO - */ - public native String toSource() /*-{ - this.toSource ? this.toSource() : "NO SOURCE"; - }-*/; - - /** * Returns a new array. */ public static native JavaScriptObject createArray() /*-{ @@ -99,6 +92,13 @@ } /** + * Call the toSource() on the JSO. + */ + public native String toSource() /*-{ + this.toSource ? this.toSource() : "NO SOURCE"; + }-*/; + + /** * catch-all toString in lieu of a better mechanism. * Basic assumption here is that this code is for debugging only! */
