[gwt-contrib] Re: Add Gears' BlobBuilder
http://galgwt-reviews.appspot.com/41603/diff/1/2
File gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java
(right):
http://galgwt-reviews.appspot.com/41603/diff/1/2#newcode43
Line 43: public void append(byte[] bytes) {
On 2009/07/06 13:26:56, zundel wrote:
> I think you could wrap up append (byte c) and this one with a
prototype like:
> append (byte... c)
> I'm not sure what drawbacks there might be.
The main drawback would be that when you call append(someByte) a new
byte[] would be instantiated!
http://galgwt-reviews.appspot.com/41603
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Add Gears' BlobBuilder
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2
File gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java
(right):
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode43
Line 43: public void append(byte[] bytes) {
On 2009/07/06 13:26:56, zundel wrote:
> I think you could wrap up append (byte c) and this one with a
prototype like:
> append (byte... c)
> I'm not sure what drawbacks there might be.
I did some checking around and maybe this is not such a good idea. The
Java tutorial on varargs says: "Generally speaking, you should not
overload a varargs method, or it will be difficult for programmers to
figure out which overloading gets called." I played around with the
ArrayHelper class writing a unit tests in a similar situation and found
that the problem isn't just with programmers, the compiler seems to get
confused as well!
http://3.latest.galgwt-reviews.appspot.com/41603
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Add Gears' BlobBuilder
Hi Thomas,
Thanks for the contribution.
I made a few suggestions and would like to see a unit test for these
classes along with the submission.
Try using 'zun...@google.com' as a reviewer.
-Eric.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2
File gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java
(right):
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode25
Line 25: public final class BlobBuilder extends JavaScriptObject {
Class needs javadoc. From the JS documentation:
A BlobBuilder is a helper object for creating Blobs. A Blob is an
immutable, read-only object, the same as a JavaScript string.
BlobBuilders are used to generate a Blob with new content. They are to
Java's StringBuilder as Gears Blobs are to Java's Strings.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode31
Line 31: public native void append(byte c) /*-{
These public methods need javadoc. We usually just copy as much as
possible from the gears JavaScript API:
"Appends bytes to the Blob-in-progress."
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode43
Line 43: public void append(byte[] bytes) {
I think you could wrap up append (byte c) and this one with a prototype
like:
append (byte... c)
I'm not sure what drawbacks there might be.
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode51
Line 51: public void append(String[] strings) {
again, append(String... strings) could catch both methods
http://3.latest.galgwt-reviews.appspot.com/41603/diff/1/2#newcode67
Line 67: private native void append(JsArrayInteger bytes) /*-{
These three could all be a single method prototyped as
private native void append(JavaScriptObject jso)
http://3.latest.galgwt-reviews.appspot.com/41603
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
[gwt-contrib] Re: Add Gears' BlobBuilder
On 6 juil, 04:09, Eric Ayers wrote: > Hi Thomas, > > It looks like something is wrong with the patch. The 'Factory' > portion doesn't show up. Well, only the side-by-side view fails actually. http://galgwt-reviews.appspot.com/41603/patch/1/4 > Probably just a hiccup with rietveld, could > you re-upload? Yep, I'll do ;-) > Also, set 'galgwt.reviews' (me, Eric Ayers) as a > reviewer I tried, but rietveld told me "galgwt.reviews" wasn't a known user !? Will re-try when re-uploading. > and re-direct the -cc to gwt-google-a...@googlegroups.com D'oh! wasn't even aware of that group! (/me sleep a bit more and read more too before starting contributing ;-) ) --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: Add Gears' BlobBuilder
Hi Thomas, It looks like something is wrong with the patch. The 'Factory' portion doesn't show up. Probably just a hiccup with rietveld, could you re-upload? Also, set 'galgwt.reviews' (me, Eric Ayers) as a reviewer and re-direct the -cc to gwt-google-a...@googlegroups.com Thanks, -Eric. On Sun, Jul 5, 2009 at 11:37 AM, Thomas Broyer wrote: > > Reviewers: t.broyer, > > Description: > This patch adds support for Gears 0.5.21 BlobBuilder API. > > Please review this at http://galgwt-reviews.appspot.com/41603 > > Affected files: > gears/src/com/google/gwt/gears/client/Factory.java > gears/src/com/google/gwt/gears/client/blobbuilder/BlobBuilder.java > gears/src/com/google/gwt/gears/client/blobbuilder/package.html > > > > -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
