Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-26 Thread Rodrigo Chandia
Yes, that was my proposal. But you are right in that putting methods with
spooky effects outside the main class may be misleading.

2010/3/26 Ray Ryan 

> If at the end of the day freezing is going to have a side effect on the
> original collection, moving the freeze call to a static method seems pretty
> mysterious to me. I think you just suggested something like this pseudocode:
>
> MutableArray myArray = new Array("able", "baker", "charlie");
> ImmutableArray solid = Collections.freeze(myArray);
> myArray.add("delta"); // assertion fails
>
> That indirection of that side effect seems worse than it would be from an
> instance method, where it's much more intuitive that I'm messing with the
> object itself.
>
> Did I misread?
>
> FWIW, I'm pretty solidly with Bruce now.
>
> rjrjr
>
>
> On Fri, Mar 26, 2010 at 9:01 AM, Rodrigo Chandia wrote:
>
>> I like having an absolutely most efficient way to take my data from a
>> mutable array into an immutable one called freeze. I also believe that
>> freezing should not be the only way to obtain immutable objects.
>>
>> I propose moving the freeze operation to a static method in Collections.
>> This is my way to say freezing should not be an inherent property of mutable
>> arrays (but support for the frozen assertions is). Building an immutable
>> collection by copy, using a builder object, or any other means should be on
>> equal footing to freezing, as an external operation.
>>
>> The implementation plan then will be (in order):
>>
>> * ImmutableArray + freezing
>> * Maps, Mutable Maps
>> * Immutable Maps + freezing
>> * Revisit builders, immutability and how it relates to operations (aka
>> Actions) such as map, filter, reduce, etc.
>> * Actions and Predicates
>>
>> Rodrigo
>>
>> 2010/3/25 Bruce Johnson 
>>
>>> What John said. JSO cross-casts allow this.
>>>
>>>
>>> On Thursday, March 25, 2010, John Tamplin  wrote:
>>> > On Thu, Mar 25, 2010 at 6:07 PM, Rodrigo Chandia 
>>> wrote:
>>> >
>>> >
>>> > (Sorry for the spam, Bruce. I forgot to press reply to all.)
>>> >
>>> > I seem to be missing some piece from the puzzle: in which way does
>>> > freezing a MutableArray prevent the allocation of an ImmutableArray
>>> > object?
>>> >
>>> > // This creates a new MutableArray instance
>>> > MutableArray ma = CollectionsFactory.createMutableArray();
>>> > ma.add("x");
>>> > ma.add("y");
>>> > ...
>>> > // But freezing will also instantiate another object, right?
>>> > ImmutableArray ia = ma.freeze();
>>> >
>>> > Internally, freeze will perform something like:
>>> >
>>> > return new ImmutableArrayImpl(elem);
>>> >
>>> > Or is it that in prod mode we can do special tricks to avoid the
>>> creation?
>>> > In Javascript, you simply cast MutableArray to ImmutableArrayImpl and
>>> return the same object.
>>> >
>>> >
>>> > --
>>> > John A. Tamplin
>>> > Software Engineer (GWT), Google
>>> >
>>> >
>>> > --
>>> > http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>> >
>>> > To unsubscribe from this group, send email to
>>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>>> this email with the words "REMOVE ME" as the subject.
>>> >
>>>
>>> --
>>> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>>
>>> To unsubscribe from this group, send email to
>>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>>> this email with the words "REMOVE ME" as the subject.
>>>
>>
>>  --
>> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>
>> To unsubscribe from this group, send email to
>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>> this email with the words "REMOVE ME" as the subject.
>>
>
>  --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-26 Thread Ray Ryan
+1 on submit early, submit often. But please cd bikeshed; ant checkstylefirst.

On Fri, Mar 26, 2010 at 11:56 AM,  wrote:

> LGTM, through the checkstyle patch #5.
>
> Given we're in bikeshed anyway, I'd submit and do a new issue/review for
> your next increment
>
>
> http://gwt-code-reviews.appspot.com/232801/show
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-26 Thread fabbott

LGTM, through the checkstyle patch #5.

Given we're in bikeshed anyway, I'd submit and do a new issue/review for
your next increment

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-26 Thread Ray Ryan
If at the end of the day freezing is going to have a side effect on the
original collection, moving the freeze call to a static method seems pretty
mysterious to me. I think you just suggested something like this pseudocode:

MutableArray myArray = new Array("able", "baker", "charlie");
ImmutableArray solid = Collections.freeze(myArray);
myArray.add("delta"); // assertion fails

That indirection of that side effect seems worse than it would be from an
instance method, where it's much more intuitive that I'm messing with the
object itself.

Did I misread?

FWIW, I'm pretty solidly with Bruce now.

rjrjr


On Fri, Mar 26, 2010 at 9:01 AM, Rodrigo Chandia wrote:

> I like having an absolutely most efficient way to take my data from a
> mutable array into an immutable one called freeze. I also believe that
> freezing should not be the only way to obtain immutable objects.
>
> I propose moving the freeze operation to a static method in Collections.
> This is my way to say freezing should not be an inherent property of mutable
> arrays (but support for the frozen assertions is). Building an immutable
> collection by copy, using a builder object, or any other means should be on
> equal footing to freezing, as an external operation.
>
> The implementation plan then will be (in order):
>
> * ImmutableArray + freezing
> * Maps, Mutable Maps
> * Immutable Maps + freezing
> * Revisit builders, immutability and how it relates to operations (aka
> Actions) such as map, filter, reduce, etc.
> * Actions and Predicates
>
> Rodrigo
>
> 2010/3/25 Bruce Johnson 
>
>> What John said. JSO cross-casts allow this.
>>
>>
>> On Thursday, March 25, 2010, John Tamplin  wrote:
>> > On Thu, Mar 25, 2010 at 6:07 PM, Rodrigo Chandia 
>> wrote:
>> >
>> >
>> > (Sorry for the spam, Bruce. I forgot to press reply to all.)
>> >
>> > I seem to be missing some piece from the puzzle: in which way does
>> > freezing a MutableArray prevent the allocation of an ImmutableArray
>> > object?
>> >
>> > // This creates a new MutableArray instance
>> > MutableArray ma = CollectionsFactory.createMutableArray();
>> > ma.add("x");
>> > ma.add("y");
>> > ...
>> > // But freezing will also instantiate another object, right?
>> > ImmutableArray ia = ma.freeze();
>> >
>> > Internally, freeze will perform something like:
>> >
>> > return new ImmutableArrayImpl(elem);
>> >
>> > Or is it that in prod mode we can do special tricks to avoid the
>> creation?
>> > In Javascript, you simply cast MutableArray to ImmutableArrayImpl and
>> return the same object.
>> >
>> >
>> > --
>> > John A. Tamplin
>> > Software Engineer (GWT), Google
>> >
>> >
>> > --
>> > http://groups.google.com/group/Google-Web-Toolkit-Contributors
>> >
>> > To unsubscribe from this group, send email to
>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>> this email with the words "REMOVE ME" as the subject.
>> >
>>
>> --
>> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>
>> To unsubscribe from this group, send email to
>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>> this email with the words "REMOVE ME" as the subject.
>>
>
>  --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-26 Thread Rodrigo Chandia
I like having an absolutely most efficient way to take my data from a
mutable array into an immutable one called freeze. I also believe that
freezing should not be the only way to obtain immutable objects.

I propose moving the freeze operation to a static method in Collections.
This is my way to say freezing should not be an inherent property of mutable
arrays (but support for the frozen assertions is). Building an immutable
collection by copy, using a builder object, or any other means should be on
equal footing to freezing, as an external operation.

The implementation plan then will be (in order):

* ImmutableArray + freezing
* Maps, Mutable Maps
* Immutable Maps + freezing
* Revisit builders, immutability and how it relates to operations (aka
Actions) such as map, filter, reduce, etc.
* Actions and Predicates

Rodrigo

2010/3/25 Bruce Johnson 

> What John said. JSO cross-casts allow this.
>
> On Thursday, March 25, 2010, John Tamplin  wrote:
> > On Thu, Mar 25, 2010 at 6:07 PM, Rodrigo Chandia 
> wrote:
> >
> >
> > (Sorry for the spam, Bruce. I forgot to press reply to all.)
> >
> > I seem to be missing some piece from the puzzle: in which way does
> > freezing a MutableArray prevent the allocation of an ImmutableArray
> > object?
> >
> > // This creates a new MutableArray instance
> > MutableArray ma = CollectionsFactory.createMutableArray();
> > ma.add("x");
> > ma.add("y");
> > ...
> > // But freezing will also instantiate another object, right?
> > ImmutableArray ia = ma.freeze();
> >
> > Internally, freeze will perform something like:
> >
> > return new ImmutableArrayImpl(elem);
> >
> > Or is it that in prod mode we can do special tricks to avoid the
> creation?
> > In Javascript, you simply cast MutableArray to ImmutableArrayImpl and
> return the same object.
> >
> >
> > --
> > John A. Tamplin
> > Software Engineer (GWT), Google
> >
> >
> > --
> > http://groups.google.com/group/Google-Web-Toolkit-Contributors
> >
> > To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
> >
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Bruce Johnson
What John said. JSO cross-casts allow this.

On Thursday, March 25, 2010, John Tamplin  wrote:
> On Thu, Mar 25, 2010 at 6:07 PM, Rodrigo Chandia  wrote:
>
>
> (Sorry for the spam, Bruce. I forgot to press reply to all.)
>
> I seem to be missing some piece from the puzzle: in which way does
> freezing a MutableArray prevent the allocation of an ImmutableArray
> object?
>
> // This creates a new MutableArray instance
> MutableArray ma = CollectionsFactory.createMutableArray();
> ma.add("x");
> ma.add("y");
> ...
> // But freezing will also instantiate another object, right?
> ImmutableArray ia = ma.freeze();
>
> Internally, freeze will perform something like:
>
> return new ImmutableArrayImpl(elem);
>
> Or is it that in prod mode we can do special tricks to avoid the creation?
> In Javascript, you simply cast MutableArray to ImmutableArrayImpl and return 
> the same object.
>
>
> --
> John A. Tamplin
> Software Engineer (GWT), Google
>
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to 
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
> email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread John Tamplin
On Thu, Mar 25, 2010 at 6:07 PM, Rodrigo Chandia wrote:

> (Sorry for the spam, Bruce. I forgot to press reply to all.)
>
> I seem to be missing some piece from the puzzle: in which way does freezing
> a MutableArray prevent the allocation of an ImmutableArray object?
>
> // This creates a new MutableArray instance
> MutableArray ma = CollectionsFactory.
> createMutableArray();
> ma.add("x");
> ma.add("y");
> ...
> // But freezing will also instantiate another object, right?
> ImmutableArray ia = ma.freeze();
>
> Internally, freeze will perform something like:
>
> return new ImmutableArrayImpl(elem);
>
> Or is it that in prod mode we can do special tricks to avoid the creation?
>

In Javascript, you simply cast MutableArray to ImmutableArrayImpl and return
the same object.

-- 
John A. Tamplin
Software Engineer (GWT), Google

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Rodrigo Chandia
(Sorry for the spam, Bruce. I forgot to press reply to all.)

I seem to be missing some piece from the puzzle: in which way does freezing
a MutableArray prevent the allocation of an ImmutableArray object?

// This creates a new MutableArray instance
MutableArray ma = CollectionsFactory.
createMutableArray();
ma.add("x");
ma.add("y");
...
// But freezing will also instantiate another object, right?
ImmutableArray ia = ma.freeze();

Internally, freeze will perform something like:

return new ImmutableArrayImpl(elem);

Or is it that in prod mode we can do special tricks to avoid the creation?


2010/3/25, Bruce Johnson :
>
> On Thursday, March 25, 2010, Freeland Abbott  wrote:
> > Am I right to think that the problem with builder.create() is that it
> implies a defensive copy, to allow you to do builder.add() afterwards, and
> we want to avoid that?  (If not, then what is the problem?)
>
>
> Yes, or to avoid having people call build() multiple times. Using a
> builder means at least two objects (builder + product(s)) for any
> array creation.
>
>
> > The solution to that could indeed be a more clever builder: at create()
> time, we return the existing array as an ImmutableArray, and let go of it in
> the builder, moving it to a previousList filed or somesuch.  If the user
> does indeed later reuse the builder with some add() (or remove() or
> whatever), we do the defensive copy then, lazily.  Presumably two
> back-to-back create() calls could just return the same list, since it's
> immutable anyway.
>
>
> These sorts of tricks usually bite us in both size and speed. Also, it
> doesn't avoid the "multiple allocations to get just one array"
> problem.
>
>
>
> >
> > I think I like that, personally.  Thoughts?
> >
> >
> >
> > On Thu, Mar 25, 2010 at 2:05 PM, Daniel Rice (דניאל רייס) <
> r...@google.com> wrote:
> >   I have the feeling that if we pushed on the API a bit we could satisfy
> all the constraints without having to do anything weird.  The only downside
> would be somewhat more interfaces to understand.  For example, the builder
> could have create() and createFrozen() methods -- a clever builder
> implementation could still manage to share state between created instances
> in some cases.
> >
> >
> >
> >
> > Dan
> >
> > On Thu, Mar 25, 2010 at 1:58 PM, Bruce Johnson  wrote:
> >
> >
> > On 3/25/10, Daniel Rice (דניאל רייס)  wrote:
> >>   I disagree with point (1).  The APIs are not the same, just almost the
> >> same.  IMHO the builder should have a freeze method while the
> MutableArray
> >> should not.  This makes it clear that freezing is a build-time
> process.  It
> >> seems to me that this could be done with a little interface inheritance
> and
> >> probably wouldn't impact generated code size at all.  Thoughts?
> >
> > (How exactly would the builder and mutable array classes differ? All
> > of the points below rest on there not being a meaningful difference.)
> >
> > I agree it's a little weird not to have a builder, but then again, it
> > would be weird to have a builder with freeze() instead of create()
> > that can be called multiple times. To me, it's worse to purport to
> > follow a pattern people assume they understand intuitively (i.e.
> > Builder in this case) but then have different functionality (freeze
> > vs. create) than to simply say, "It's unusual, but easy to learn and
> > maximally efficient".
> >
> > And there's also an argument to be made that fewer classes are easier,
> > all else being equal. Which would favor not having a builder that is
> > quite redundant to mutable array.
> >
> > -- Bruce
> >
> > --
> > http://groups.google.com/group/Google-Web-Toolkit-Contributors
> >
> > To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
> >
> >
> >
> >
> >
> >
> >
> > --
> > http://groups.google.com/group/Google-Web-Toolkit-Contributors
> >
> > To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
> >
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Bruce Johnson
On Thursday, March 25, 2010, Freeland Abbott  wrote:
> Am I right to think that the problem with builder.create() is that it implies 
> a defensive copy, to allow you to do builder.add() afterwards, and we want to 
> avoid that?  (If not, then what is the problem?)

Yes, or to avoid having people call build() multiple times. Using a
builder means at least two objects (builder + product(s)) for any
array creation.

> The solution to that could indeed be a more clever builder: at create() time, 
> we return the existing array as an ImmutableArray, and let go of it in the 
> builder, moving it to a previousList filed or somesuch.  If the user does 
> indeed later reuse the builder with some add() (or remove() or whatever), we 
> do the defensive copy then, lazily.  Presumably two back-to-back create() 
> calls could just return the same list, since it's immutable anyway.

These sorts of tricks usually bite us in both size and speed. Also, it
doesn't avoid the "multiple allocations to get just one array"
problem.


>
> I think I like that, personally.  Thoughts?
>
>
>
> On Thu, Mar 25, 2010 at 2:05 PM, Daniel Rice (דניאל רייס)  
> wrote:
>   I have the feeling that if we pushed on the API a bit we could satisfy all 
> the constraints without having to do anything weird.  The only downside would 
> be somewhat more interfaces to understand.  For example, the builder could 
> have create() and createFrozen() methods -- a clever builder implementation 
> could still manage to share state between created instances in some cases.
>
>
>
>
> Dan
>
> On Thu, Mar 25, 2010 at 1:58 PM, Bruce Johnson  wrote:
>
>
> On 3/25/10, Daniel Rice (דניאל רייס)  wrote:
>>   I disagree with point (1).  The APIs are not the same, just almost the
>> same.  IMHO the builder should have a freeze method while the MutableArray
>> should not.  This makes it clear that freezing is a build-time process.  It
>> seems to me that this could be done with a little interface inheritance and
>> probably wouldn't impact generated code size at all.  Thoughts?
>
> (How exactly would the builder and mutable array classes differ? All
> of the points below rest on there not being a meaningful difference.)
>
> I agree it's a little weird not to have a builder, but then again, it
> would be weird to have a builder with freeze() instead of create()
> that can be called multiple times. To me, it's worse to purport to
> follow a pattern people assume they understand intuitively (i.e.
> Builder in this case) but then have different functionality (freeze
> vs. create) than to simply say, "It's unusual, but easy to learn and
> maximally efficient".
>
> And there's also an argument to be made that fewer classes are easier,
> all else being equal. Which would favor not having a builder that is
> quite redundant to mutable array.
>
> -- Bruce
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to 
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
> email with the words "REMOVE ME" as the subject.
>
>
>
>
>
>
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to 
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
> email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Lex Spoon
On Thu, Mar 25, 2010 at 2:16 PM, Freeland Abbott  wrote:

> Am I right to think that the problem with builder.create() is that it
> implies a defensive copy, to allow you to do builder.add() afterwards, and
> we want to avoid that?  (If not, then what is the problem?)
>
> The solution to that could indeed be a more clever builder: at create()
> time, we return the existing array as an ImmutableArray, *and let go of it
> in the builder, moving it to a previousList filed or somesuch.*  If the
> user does indeed later reuse the builder with some add() (or remove() or
> whatever), we do the defensive copy then, lazily.  Presumably two
> back-to-back create() calls could just return the same list, since it's
> immutable anyway.
>

That works fine

It won't always optimize as well, though.  For a builder created in one
method, built up, and then turned into an immutable collection, anything
goes.  Unwrap the fields of the builder object, inline the add/remove/etc
methods, and use data flow for the rest.  However, for a builder passed
around to multiple methods, this looks much harder, and surely the optimizer
won't always figure it out.

For freeze, I presume it works out that in the prod-mode version the frozen
collection really is the same object?  In that case, we should get the
tightest possible code just by inlining add/remove/etc.  There isn't any
fancy inference needed to prove that add() is never called after create().

-Lex

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Freeland Abbott
Am I right to think that the problem with builder.create() is that it
implies a defensive copy, to allow you to do builder.add() afterwards, and
we want to avoid that?  (If not, then what is the problem?)

The solution to that could indeed be a more clever builder: at create()
time, we return the existing array as an ImmutableArray, *and let go of it
in the builder, moving it to a previousList filed or somesuch.*  If the user
does indeed later reuse the builder with some add() (or remove() or
whatever), we do the defensive copy then, lazily.  Presumably two
back-to-back create() calls could just return the same list, since it's
immutable anyway.

I think I like that, personally.  Thoughts?



On Thu, Mar 25, 2010 at 2:05 PM, Daniel Rice (דניאל רייס)
wrote:

>   I have the feeling that if we pushed on the API a bit we could satisfy
> all the constraints without having to do anything weird.  The only downside
> would be somewhat more interfaces to understand.  For example, the builder
> could have create() and createFrozen() methods -- a clever builder
> implementation could still manage to share state between created instances
> in some cases.
>
> Dan
>
> On Thu, Mar 25, 2010 at 1:58 PM, Bruce Johnson  wrote:
>
>> On 3/25/10, Daniel Rice (דניאל רייס)  wrote:
>> >   I disagree with point (1).  The APIs are not the same, just almost the
>> > same.  IMHO the builder should have a freeze method while the
>> MutableArray
>> > should not.  This makes it clear that freezing is a build-time process.
>>  It
>> > seems to me that this could be done with a little interface inheritance
>> and
>> > probably wouldn't impact generated code size at all.  Thoughts?
>>
>> (How exactly would the builder and mutable array classes differ? All
>> of the points below rest on there not being a meaningful difference.)
>>
>> I agree it's a little weird not to have a builder, but then again, it
>> would be weird to have a builder with freeze() instead of create()
>> that can be called multiple times. To me, it's worse to purport to
>> follow a pattern people assume they understand intuitively (i.e.
>> Builder in this case) but then have different functionality (freeze
>> vs. create) than to simply say, "It's unusual, but easy to learn and
>> maximally efficient".
>>
>> And there's also an argument to be made that fewer classes are easier,
>> all else being equal. Which would favor not having a builder that is
>> quite redundant to mutable array.
>>
>> -- Bruce
>>
>> --
>>
>> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>
>> To unsubscribe from this group, send email to
>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>> this email with the words "REMOVE ME" as the subject.
>>
>
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread דניאל רייס
  I have the feeling that if we pushed on the API a bit we could satisfy all
the constraints without having to do anything weird.  The only downside
would be somewhat more interfaces to understand.  For example, the builder
could have create() and createFrozen() methods -- a clever builder
implementation could still manage to share state between created instances
in some cases.

Dan

On Thu, Mar 25, 2010 at 1:58 PM, Bruce Johnson  wrote:

> On 3/25/10, Daniel Rice (דניאל רייס)  wrote:
> >   I disagree with point (1).  The APIs are not the same, just almost the
> > same.  IMHO the builder should have a freeze method while the
> MutableArray
> > should not.  This makes it clear that freezing is a build-time process.
>  It
> > seems to me that this could be done with a little interface inheritance
> and
> > probably wouldn't impact generated code size at all.  Thoughts?
>
> (How exactly would the builder and mutable array classes differ? All
> of the points below rest on there not being a meaningful difference.)
>
> I agree it's a little weird not to have a builder, but then again, it
> would be weird to have a builder with freeze() instead of create()
> that can be called multiple times. To me, it's worse to purport to
> follow a pattern people assume they understand intuitively (i.e.
> Builder in this case) but then have different functionality (freeze
> vs. create) than to simply say, "It's unusual, but easy to learn and
> maximally efficient".
>
> And there's also an argument to be made that fewer classes are easier,
> all else being equal. Which would favor not having a builder that is
> quite redundant to mutable array.
>
> -- Bruce
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Bruce Johnson
On 3/25/10, Daniel Rice (דניאל רייס)  wrote:
>   I disagree with point (1).  The APIs are not the same, just almost the
> same.  IMHO the builder should have a freeze method while the MutableArray
> should not.  This makes it clear that freezing is a build-time process.  It
> seems to me that this could be done with a little interface inheritance and
> probably wouldn't impact generated code size at all.  Thoughts?

(How exactly would the builder and mutable array classes differ? All
of the points below rest on there not being a meaningful difference.)

I agree it's a little weird not to have a builder, but then again, it
would be weird to have a builder with freeze() instead of create()
that can be called multiple times. To me, it's worse to purport to
follow a pattern people assume they understand intuitively (i.e.
Builder in this case) but then have different functionality (freeze
vs. create) than to simply say, "It's unusual, but easy to learn and
maximally efficient".

And there's also an argument to be made that fewer classes are easier,
all else being equal. Which would favor not having a builder that is
quite redundant to mutable array.

-- Bruce

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread rchandia

I have just uploaded a couple of checkstyle fixes for a quick review.

I'll get to the ImmutableArray implementation now. It seems the builder
approach has merits. I'd like to give it a little more thought.

On 2010/03/25 16:39:42, rchandia wrote:




http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread rchandia

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread דניאל רייס
  I disagree with point (1).  The APIs are not the same, just almost the
same.  IMHO the builder should have a freeze method while the MutableArray
should not.  This makes it clear that freezing is a build-time process.  It
seems to me that this could be done with a little interface inheritance and
probably wouldn't impact generated code size at all.  Thoughts?

Dan

On Mon, Mar 22, 2010 at 3:13 PM, Bruce Johnson  wrote:

> Here's how freeze() got introduced.
>
> You need to be able to have ImmutableArray without any mutators, and you
> need to be able to create them, thus you need a builder. A very frequent
> pattern will be to build up an array with a builder (the hypothetical
> ImmutableArrayBuilder) and then want to get an ImmutableArray from it. So,
> you'd often write this:
>
> == BEGIN SNIPPET ==
> ImmutableArrayBuilder b = new ImmutableArrayBuilder();
> b.add(...);
> ...
> ImmutableArray ia = b.build();
>
> // throw away b
> == END SNIPPET ==
>
> A few observations about the above inevitable code snippet:
> 1) ImmutableArrayBuilder would have an API that's almost the same as
> MutableArray, so why make it a separate class?
> 2) The builder pattern typically allows multiple objects to be built from
> the same builder, but it's easy to imagine that typical uses won't actually
> want to reuse builder instances, thus we're paying for twice the number of
> object allocations (1 builder + 1 product) for the common case.
>
> Why solve it with freeze()? This design makes MutableArray into a builder
> of ImmutableArrays that is extremely efficient (since it will be implemented
> as "return this" and will use JSO cross-casting), thus avoiding the wasteful
> builder allocation problem. Secondly, the "freeze()" semantics don't open up
> the notion that a single MutableArray could be used as a reusable factory --
> unlike build(), freeze() makes it obvious that you can't mess with the
> builder anymore.
>
> Hope that makes sense. As Freeland said, this design is based on the idea
> of maximum performance, minimum size, and a set of types that allows
> applications to avoid expensive allocations and copying. We need a rich
> enough set of types that, whatever the circumstances, you're never more than
> cheap (hopefully O(1)-time) operation away from having an object that
> satisfies the need. For example, we want to encourage handing out aliases to
> private fields (making it safe to do so by providing a read-only handle in
> the form of the root type).
>
> On Mon, Mar 22, 2010 at 2:06 PM, Ray Ryan  wrote:
>
>> Can you outline a use case? I don't get it. My argument isn't with
>> isFrozen, it's with the freezing feature per se. I can't see a reasonable
>> use for it.
>>
>>
>> On Mon, Mar 22, 2010 at 11:03 AM, Rodrigo Chandia wrote:
>>
>>> isFrozen allows assertions on the status of a mutable collection. During
>>> normal use (assertions disabled), there should be no need to call isFrozen.
>>> Moreover, using isFrozen outside of an assertion, or while assertions are
>>> disabled, is not guaranteed to work at all. The intention is to avoid having
>>> to pay a runtime penalty and discourage defensive programming.
>>>
>>> Related to the review, it was not my intention to introduce isFrozen just
>>> yet (but it slipped through, sorry). isFrozen is a construct that only makes
>>> sense when when Immutable classes are introduced, along with assertions and
>>> tests relevant to immutability. At this point I wanted to concentrate on the
>>> Mutable and the parent Read-only classes.
>>>
>>> Is isFrozen still a bad idea when used for assertions only?
>>>
>>>
>>> 2010/3/22 
>>>
>>> Can someone explain why isFrozen is a good idea? It sounds really,
 really bad to me.


 http://gwt-code-reviews.appspot.com/232801/show

>>>
>>>
>>
>  --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread Rodrigo Chandia
Thx!

2010/3/25 

> LGTM
>
>
> http://gwt-code-reviews.appspot.com/232801/show
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-25 Thread fabbott

LGTM

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-23 Thread rchandia

On 2010/03/23 19:31:39, fabbott wrote:

Can you edit the issue, to cc mailto:br...@google.com vice unqualified

bruce?

Oh, I almost forgot. I have tried to fix the cc list but whatever I
enter when I edit the issue is lost in the ether.

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-23 Thread rchandia

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-23 Thread rchandia


http://gwt-code-reviews.appspot.com/232801/diff/25001/26007
File bikeshed/src/com/google/gwt/collections/MutableArray.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/25001/26007#newcode112
bikeshed/src/com/google/gwt/collections/MutableArray.java:112: elems =
newElems;
On 2010/03/23 19:31:39, fabbott wrote:

Does this set you up badly after the length 1 to length 0 transition,

i.e.

removing the last element?  It seems _likely_ to work as-is, but is

worth (a) a

test case, and (b) maybe restoring elem==null as an invariant.


Returning back to elem==null seems cleaner to me too.

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008
File bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java
(right):

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode35
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:35:
assertionsEnabled = true;
On 2010/03/23 19:31:39, fabbott wrote:

this is okay, but I didn't have a problem with desiredAssertionStatus

(which I

think has the general advantage of being compile-time constant).


Changed to use desiredAssertionStatus

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode41
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:41: return
"com.google.gwt.collections.Collections";
On 2010/03/23 19:31:39, fabbott wrote:

These mean different things; which do you want?



return null tests as a vanilla JUnit test, which I thought met your

"Java only"

incremental statement.  Return string tests in GWT, i.e. as client

code, either

java (devmode) or JS.  You're probably fine with this change, but

they're not

synonymous.



I'd expected, from the first, that you'd derive a JsObjectArrayTest

and override

to return string, so you'd have two tests one as server Java and one

as client

Java/JS.  Which may be overkill, server java and client java being not

so much

different, but the server-java test should run faster without our

startup

overhead.


Oops. I was testing null vs. "com...Collections" and did not notice I
had not reverted it to null. I think it will be important to have both
sets of tests. These classes are meant to be used both on server and
client sides.

But null is the right option here, I think.

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode115
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:115:
On 2010/03/23 19:31:39, fabbott wrote:

add a add/remove/add test for the 1->0 transition and recovery.


Done.

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-23 Thread fabbott

Looks almost good to me; I think you're non-invariant (but maybe
correct) if you remove the last element, so that needs a test, and you
may have been better off with getModuleName() returning null, depending
on what you're trying to test in this increment.

Can you edit the issue, to cc br...@google.com vice unqualified bruce?


http://gwt-code-reviews.appspot.com/232801/diff/25001/26007
File bikeshed/src/com/google/gwt/collections/MutableArray.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/25001/26007#newcode112
bikeshed/src/com/google/gwt/collections/MutableArray.java:112: elems =
newElems;
Does this set you up badly after the length 1 to length 0 transition,
i.e. removing the last element?  It seems _likely_ to work as-is, but is
worth (a) a test case, and (b) maybe restoring elem==null as an
invariant.

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008
File bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java
(right):

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode35
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:35:
assertionsEnabled = true;
this is okay, but I didn't have a problem with desiredAssertionStatus
(which I think has the general advantage of being compile-time
constant).

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode41
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:41: return
"com.google.gwt.collections.Collections";
These mean different things; which do you want?

return null tests as a vanilla JUnit test, which I thought met your
"Java only" incremental statement.  Return string tests in GWT, i.e. as
client code, either java (devmode) or JS.  You're probably fine with
this change, but they're not synonymous.

I'd expected, from the first, that you'd derive a JsObjectArrayTest and
override to return string, so you'd have two tests one as server Java
and one as client Java/JS.  Which may be overkill, server java and
client java being not so much different, but the server-java test should
run faster without our startup overhead.

http://gwt-code-reviews.appspot.com/232801/diff/25001/26008#newcode115
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:115:
add a add/remove/add test for the 1->0 transition and recovery.

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-23 Thread rchandia

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-23 Thread rchandia


http://gwt-code-reviews.appspot.com/232801/diff/3001/4002
File bikeshed/src/com/google/gwt/collections/Assertions.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4002#newcode24
bikeshed/src/com/google/gwt/collections/Assertions.java:24: assert
(index >= 0 && index < maxExclusive) : msgIndexOutOfRange(index,
minInclusive,
On 2010/03/22 18:38:53, Dan Rice wrote:

minInclusive is never checked here


Done.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4002#newcode37
bikeshed/src/com/google/gwt/collections/Assertions.java:37: static
String msgShouldNotBeNull() {
On 2010/03/22 15:59:55, fabbott wrote:

I'm not entirely sure these are useful (vice just using the constants

in-line in

the sole call sites), but okay...

Let's inline it and refactor later if there is a need for it.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007
File bikeshed/src/com/google/gwt/collections/MutableArray.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode23
bikeshed/src/com/google/gwt/collections/MutableArray.java:23: // Used
when there is exactly one element in progress
On 2010/03/22 15:59:55, fabbott wrote:

"in progress" doesn't mean much for me here.


Done. Removed "in progress"

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode38
bikeshed/src/com/google/gwt/collections/MutableArray.java:38:
@SuppressWarnings("unchecked")
On 2010/03/22 15:59:55, fabbott wrote:

what's the unchecked warning from?


Unused. Removed.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode56
bikeshed/src/com/google/gwt/collections/MutableArray.java:56: } else if
(singleElem != null) {
On 2010/03/22 15:59:55, fabbott wrote:

two questions: first, is the whole singleElem special case actually

gaining

anything here?  second, am I allowed to have an array of size 1

containing only

null?  (And if not, why not, and how do I know?)

It seems overly complex. I removed the single element case.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode75
bikeshed/src/com/google/gwt/collections/MutableArray.java:75:
On 2010/03/22 15:59:55, fabbott wrote:

I think you need a check here (and remove and set) against isfrozen?


Yes, I will add the isFrozen checks at the same time I add the immutable
functionality. It is just that I wanted to begin by reviewing the
Mutable and Read-only cases.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode79
bikeshed/src/com/google/gwt/collections/MutableArray.java:79: //
TODO(bruce): grow smartly
On 2010/03/22 15:59:55, fabbott wrote:

not likely to be bruce.  (Not sure we attribute our todo's generally,

anyway.)
Fixed and made more explicit.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode92
bikeshed/src/com/google/gwt/collections/MutableArray.java:92: singleElem
= elem;
On 2010/03/22 15:59:55, fabbott wrote:

you lose if elem==null

Removed the singleElem contruct. I will revisit the idea when we get to
benchmarking and optimization.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode110
bikeshed/src/com/google/gwt/collections/MutableArray.java:110: // Bad!
But only happens if index is bad and assertions are off
On 2010/03/22 15:59:55, fabbott wrote:

I know bruce was imagining minimal safety overhead, but I'd like to

see an

assertion here, just in case we're someday wrong/buggy about the

precondition

assertions.  I can live with assertions-only correctness checking, but

get

queasy about deliberate holes.

Added assert(false) to force an assert if this condition is ever hit.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode131
bikeshed/src/com/google/gwt/collections/MutableArray.java:131: // Bad!
But only happens if index is bad and assertions are off
On 2010/03/22 15:59:55, fabbott wrote:

as above, wrt assertions for checking.


Done.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008
File bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java
(right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode29
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:29: throw
new IllegalStateException("This test case requires assertions to be
enabled");
On 2010/03/22 15:59:55, fabbott wrote:

um.  I dunno that this should be true, or if true I think we might

want it to

pass (but be a no-op) rather than fail when run without 'em.


Added guards to disable tests that depend on assertion when assertion is
disabled.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode108
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:108:
b.add(null);
On 2010/03/22 15:59:55, fabbott wrote:

yeah, try doing this (add of null) as your first item. ;-)


Done.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode126
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:126:
b.set(3, "eclair");
On 2010/03/22 15:59:55, fabbott wrote:

check result?


Done.

http://gwt-code-reviews.appspot

Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Ray Cromwell
There is also a chance that the compiler can be taught about Immutable
types to help in analysis. A typical case I've seen in record based
function languages is an optimization called equational reasoning,
which essentially boils down to statically determining the the fields
of an object as constant values, and then substituting those values
where appropriate, essentially treating the object as nothing more
than a compile time macro holding a few values.

Consider:

ImmutablePerson p = new ImmutablePerson("Ray", "Cromwell");

salute(p.firstName, p.lastName) => salute("Ray", "Cromwell");

or better,

ImmutableArray ar = ImmutableArray.cons(1,2,3,4,5);

add(ar.get(2), ar.get(3)) => add(3, 4) => 7

It's true that the compiler in theory could do constant analysis/copy
propagation, but it tends to get difficult when the intermediate
values are passed to other methods and you don't know who is modifying
them The immutability of these types let's the compiler assume that
regardless of which methods receive these objects, they cannot
possibly be modified.


On Mon, Mar 22, 2010 at 4:05 PM, Bruce Johnson  wrote:
> I think Rodrigo's point already subsumed what I'm about to say, but there
> are three cases here:
> 1) A read-only reference to a collection that may or may not be mutable by
> someone else. This is the purpose of the root type Array, which has not
> mutators but doesn't make a guarantee about whether the referenced
> collection is mutable or immutable.
> 2) A reference to a read-only collection whose contents are guaranteed to in
> fact be immutable.
> 3) A normal, mutable array that is read/write.
> @Ray: The idea of an immutableView() method doesn't seem to quite handle
> these use cases, because either the returned object is a separate object
> with a runtime wrapper (the equivalent of unmodifiableList()) or it's a
> read-only interface to a collection that isn't really guaranteed to be
> immutable, merely read-only. Also note that MutableArray extends the
> read-only Array class, and so every MutableArray can be implicitly returned
> as a read-only alias anywhere you want, which is even easier than
> immutableView().
> I would guess it would be fairly typical to use mostly MutableArray and
> Array, but ImmutableArray has a real role, too. Imagine a MyPanel that takes
> a list of Widgets in its constructor. You could declare the constructor like
> this:
>    public MyPanel(ImmutableArray widgets) { ... }
> That's useful because you can code the class with full knowledge that the
> set of widgets won't be changing out from under you from the calling code --
> you can just directly assign that param to a field without making a copy.
> Had the constructor been:
>    public MyPanel(List widgets) { ... }
> you could never be sure and would probably feel compelled to make a
> defensive copy.
> I'm going to guess this is how it would play out:
> 1) Constructors will often take ImmutableArray because it's nice to assign
> ctor params directly to fields and know for a fact there's no reason to make
> defensive copies.
> 2) Fields will be declared as MutableArray, and in getters the return type
> will be Array, thus allowing getters to safely return references directly,
> happy in the knowledge that there is neither a need to make a defensive
> outgoing copy nor a risk that the caller can mutate your private field's
> contents.
> 3) Library methods will take the least-specific type that can suffice for
> the respective algorithm. For example, binarySearch(), find(), forEach(),
> etc. would take a param of type Array, thus allowing either mutable or
> immutable arrays to use the method. Then again, sort() would explicitly take
> a MutableArray.
> And of course, there is no desire to design all this in a vacuum -- and we
> shouldn't argue against it in a vacuum either. This is a starting point
> strawman, so I'd propose we use this design as a stake in the ground and
> then see how it would play it if we retrofitted it into real code, both app
> code and library code. Then we'll really get a feel for how useful vs.
> confusing it is, and most importantly, we can get metrics on the speed and
> size.
>
>
> On Mon, Mar 22, 2010 at 6:34 PM, Rodrigo Chandia 
> wrote:
>>
>> Immutability is a stronger assertion than read-only access. If I receive a
>> read-only object I better make sure to handle the case of the data being
>> changed by others; be it by tacit agreement, using other channels, locking
>> or simply ignoring the issue. Immutability guarantees the data is stable and
>> unchanging. The agreement is explicit. I will be able to read it to my
>> heart's content without worry.
>> Granted, being this a simple implementation it has some inconveniences:
>> 1. The Immutability is achieved by "freezing" the original Mutable
>> container while it may be useful to keep working with the original container
>> instead.
>> 2. The receiver of a MutableArray may find later that someone called
>> freeze on the object causing a

Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Ray Ryan
  public MyPanel(ImmutableArray widgets) { ... }

That's the use case I was missing. Thanks for taking the time to debate,
guys.

rjrjr

On Mon, Mar 22, 2010 at 4:05 PM, Bruce Johnson  wrote:

> I think Rodrigo's point already subsumed what I'm about to say, but there
> are three cases here:
>
> 1) A read-only reference to a collection that may or may not be mutable by
> someone else. This is the purpose of the root type Array, which has not
> mutators but doesn't make a guarantee about whether the referenced
> collection is mutable or immutable.
>
> 2) A reference to a read-only collection whose contents are guaranteed to
> in fact be immutable.
>
> 3) A normal, mutable array that is read/write.
>
> @Ray: The idea of an immutableView() method doesn't seem to quite handle
> these use cases, because either the returned object is a separate object
> with a runtime wrapper (the equivalent of unmodifiableList()) or it's a
> read-only interface to a collection that isn't really guaranteed to be
> immutable, merely read-only. Also note that MutableArray extends the
> read-only Array class, and so every MutableArray can be implicitly returned
> as a read-only alias anywhere you want, which is even easier than
> immutableView().
>
> I would guess it would be fairly typical to use mostly MutableArray and
> Array, but ImmutableArray has a real role, too. Imagine a MyPanel that takes
> a list of Widgets in its constructor. You could declare the constructor like
> this:
>
>public MyPanel(ImmutableArray widgets) { ... }
>
> That's useful because you can code the class with full knowledge that the
> set of widgets won't be changing out from under you from the calling code --
> you can just directly assign that param to a field without making a copy.
> Had the constructor been:
>
>public MyPanel(List widgets) { ... }
>
> you could never be sure and would probably feel compelled to make a
> defensive copy.
>
> I'm going to guess this is how it would play out:
>
> 1) Constructors will often take ImmutableArray because it's nice to assign
> ctor params directly to fields and know for a fact there's no reason to make
> defensive copies.
>
> 2) Fields will be declared as MutableArray, and in getters the return type
> will be Array, thus allowing getters to safely return references directly,
> happy in the knowledge that there is neither a need to make a defensive
> outgoing copy nor a risk that the caller can mutate your private field's
> contents.
>
> 3) Library methods will take the least-specific type that can suffice for
> the respective algorithm. For example, binarySearch(), find(), forEach(),
> etc. would take a param of type Array, thus allowing either mutable or
> immutable arrays to use the method. Then again, sort() would explicitly take
> a MutableArray.
>
> And of course, there is no desire to design all this in a vacuum -- and we
> shouldn't argue against it in a vacuum either. This is a starting point
> strawman, so I'd propose we use this design as a stake in the ground and
> then see how it would play it if we retrofitted it into real code, both app
> code and library code. Then we'll really get a feel for how useful vs.
> confusing it is, and most importantly, we can get metrics on the speed and
> size.
>
>
>
> On Mon, Mar 22, 2010 at 6:34 PM, Rodrigo Chandia wrote:
>
>> Immutability is a stronger assertion than read-only access. If I receive a
>> read-only object I better make sure to handle the case of the data being
>> changed by others; be it by tacit agreement, using other channels, locking
>> or simply ignoring the issue. Immutability guarantees the data is stable and
>> unchanging. The agreement is explicit. I will be able to read it to my
>> heart's content without worry.
>>
>> Granted, being this a simple implementation it has some inconveniences:
>>
>> 1. The Immutability is achieved by "freezing" the original Mutable
>> container while it may be useful to keep working with the original container
>> instead.
>> 2. The receiver of a MutableArray may find later that someone called
>> freeze on the object causing an error.
>>
>> This is largely a matter of it being a lightweight implementation. There
>> is no contract forcing the mutable view to be tied to the original mutable
>> container. A more sophisticated implementation could copy the container,
>> implement copy-on-write semantics, or any number of alternative
>> implementations. Still the meaning of an immutable collection implies the
>> contents will not change, while a read-only view makes no such promise.
>>
>> 2010/3/22 Ray Ryan 
>>
>>> My argument is that one is necessary and sufficient. Two is kind of
>>> pointless if you have achieved one, and maybe even counterproductive.
>>>
>>>
>>> On Mon, Mar 22, 2010 at 2:32 PM, Joel Webber  wrote:
>>>
 I think we're talking about two different things here. Rodrigo's (valid)
 point is that implementing immutability sanely early on is a good idea. And
 this implementation is pret

Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Bruce Johnson
I think Rodrigo's point already subsumed what I'm about to say, but there
are three cases here:

1) A read-only reference to a collection that may or may not be mutable by
someone else. This is the purpose of the root type Array, which has not
mutators but doesn't make a guarantee about whether the referenced
collection is mutable or immutable.

2) A reference to a read-only collection whose contents are guaranteed to in
fact be immutable.

3) A normal, mutable array that is read/write.

@Ray: The idea of an immutableView() method doesn't seem to quite handle
these use cases, because either the returned object is a separate object
with a runtime wrapper (the equivalent of unmodifiableList()) or it's a
read-only interface to a collection that isn't really guaranteed to be
immutable, merely read-only. Also note that MutableArray extends the
read-only Array class, and so every MutableArray can be implicitly returned
as a read-only alias anywhere you want, which is even easier than
immutableView().

I would guess it would be fairly typical to use mostly MutableArray and
Array, but ImmutableArray has a real role, too. Imagine a MyPanel that takes
a list of Widgets in its constructor. You could declare the constructor like
this:

   public MyPanel(ImmutableArray widgets) { ... }

That's useful because you can code the class with full knowledge that the
set of widgets won't be changing out from under you from the calling code --
you can just directly assign that param to a field without making a copy.
Had the constructor been:

   public MyPanel(List widgets) { ... }

you could never be sure and would probably feel compelled to make a
defensive copy.

I'm going to guess this is how it would play out:

1) Constructors will often take ImmutableArray because it's nice to assign
ctor params directly to fields and know for a fact there's no reason to make
defensive copies.

2) Fields will be declared as MutableArray, and in getters the return type
will be Array, thus allowing getters to safely return references directly,
happy in the knowledge that there is neither a need to make a defensive
outgoing copy nor a risk that the caller can mutate your private field's
contents.

3) Library methods will take the least-specific type that can suffice for
the respective algorithm. For example, binarySearch(), find(), forEach(),
etc. would take a param of type Array, thus allowing either mutable or
immutable arrays to use the method. Then again, sort() would explicitly take
a MutableArray.

And of course, there is no desire to design all this in a vacuum -- and we
shouldn't argue against it in a vacuum either. This is a starting point
strawman, so I'd propose we use this design as a stake in the ground and
then see how it would play it if we retrofitted it into real code, both app
code and library code. Then we'll really get a feel for how useful vs.
confusing it is, and most importantly, we can get metrics on the speed and
size.



On Mon, Mar 22, 2010 at 6:34 PM, Rodrigo Chandia wrote:

> Immutability is a stronger assertion than read-only access. If I receive a
> read-only object I better make sure to handle the case of the data being
> changed by others; be it by tacit agreement, using other channels, locking
> or simply ignoring the issue. Immutability guarantees the data is stable and
> unchanging. The agreement is explicit. I will be able to read it to my
> heart's content without worry.
>
> Granted, being this a simple implementation it has some inconveniences:
>
> 1. The Immutability is achieved by "freezing" the original Mutable
> container while it may be useful to keep working with the original container
> instead.
> 2. The receiver of a MutableArray may find later that someone called freeze
> on the object causing an error.
>
> This is largely a matter of it being a lightweight implementation. There is
> no contract forcing the mutable view to be tied to the original mutable
> container. A more sophisticated implementation could copy the container,
> implement copy-on-write semantics, or any number of alternative
> implementations. Still the meaning of an immutable collection implies the
> contents will not change, while a read-only view makes no such promise.
>
> 2010/3/22 Ray Ryan 
>
>> My argument is that one is necessary and sufficient. Two is kind of
>> pointless if you have achieved one, and maybe even counterproductive.
>>
>>
>> On Mon, Mar 22, 2010 at 2:32 PM, Joel Webber  wrote:
>>
>>> I think we're talking about two different things here. Rodrigo's (valid)
>>> point is that implementing immutability sanely early on is a good idea. And
>>> this implementation is pretty much analogous to the one you describe from
>>> Cocoa.
>>>
>>> The question at hand is whether it makes sense to get an immutable
>>> collection from a mutable one, with no copies. There are two ways to do
>>> this:
>>> 1. Create an immutable "view" of a mutable list, but with no guarantees
>>> that the list won't be mutated by the original 

Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Rodrigo Chandia
Immutability is a stronger assertion than read-only access. If I receive a
read-only object I better make sure to handle the case of the data being
changed by others; be it by tacit agreement, using other channels, locking
or simply ignoring the issue. Immutability guarantees the data is stable and
unchanging. The agreement is explicit. I will be able to read it to my
heart's content without worry.

Granted, being this a simple implementation it has some inconveniences:

1. The Immutability is achieved by "freezing" the original Mutable container
while it may be useful to keep working with the original container instead.
2. The receiver of a MutableArray may find later that someone called freeze
on the object causing an error.

This is largely a matter of it being a lightweight implementation. There is
no contract forcing the mutable view to be tied to the original mutable
container. A more sophisticated implementation could copy the container,
implement copy-on-write semantics, or any number of alternative
implementations. Still the meaning of an immutable collection implies the
contents will not change, while a read-only view makes no such promise.

2010/3/22 Ray Ryan 

> My argument is that one is necessary and sufficient. Two is kind of
> pointless if you have achieved one, and maybe even counterproductive.
>
>
> On Mon, Mar 22, 2010 at 2:32 PM, Joel Webber  wrote:
>
>> I think we're talking about two different things here. Rodrigo's (valid)
>> point is that implementing immutability sanely early on is a good idea. And
>> this implementation is pretty much analogous to the one you describe from
>> Cocoa.
>>
>> The question at hand is whether it makes sense to get an immutable
>> collection from a mutable one, with no copies. There are two ways to do
>> this:
>> 1. Create an immutable "view" of a mutable list, but with no guarantees
>> that the list won't be mutated by the original owner later.
>> 2. "Freeze" a mutable list into an immutable view of said list, making the
>> former "runtime immutable".
>>
>> (1) solves the problem of a class giving access to one of its internal
>> collections without having to guard against external mutation. (2) can be
>> used to replicate the "builder" pattern.
>>
>> I don't have a strong opinion about (2) myself, but (1) is pretty damned
>> important, because it's the source of innumerable stupid defensive copies in
>> JRE code. The provider of such an interface would just have to be very clear
>> about whether the "immutable" list might be modified later (because it's a
>> view on a mutable one).
>>
>> On Mon, Mar 22, 2010 at 5:23 PM, Ray Ryan  wrote:
>>
>>> I think you're missing my point. An object is immutable if there exists
>>> no api to mutate it. That should be enough.
>>>
>>> Let me put it another way. It's lame that the JRE achieves immutability
>>> by turning mutate methods into runtime errors. It will be equally lame of us
>>> to do the same, especially since we can't enforce it at production time. It
>>> would be much better to provide an api such that there is not even possible
>>> to compile the mutate call (without cheating with casts, but then you know
>>> you're being bad).
>>>
>>> The Cocoa approach to this is to have interfaces like NSArray be
>>> immutable, and then have NSMutableArray extend NSArray. If we're going to
>>> roll our own collection classes, it seems to me we could do the same: e.g.
>>> LiteImmutableList and List extends LiteImmutableList.
>>>
>>> rjrjr
>>>
>>>
>>> On Mon, Mar 22, 2010 at 2:12 PM, Rodrigo Chandia wrote:
>>>
 I like the *concept* of immutability being introduced early in the
 development. The initial implementation may be limiting for some use cases,
 but I believe it is a useful concept to expand on. If specific needs 
 require
 simultaneous mutable and immutable access we can provide implementations to
 address that problem (copy on write semantics, for example).

 2010/3/22 Ray Ryan 

 I guess I'm overstating my opposition. It's not really dangerous, but it
> just doesn't seem useful. Just by existing I think it'll promote confusion
> and perhaps bad habits. Why bother?
>
> I think the 90% use case is for something like the following (writing
> in JRE terms here):
>
> private final List magicValues;
> {
>List buildValues = new ArrayList();
>buildValues.add("able");
>buildValues.add("baker");
>buildValues.add("charlie");
>magicValues = Collections.unmodifiableList(buildValues);
> }
>
> Ta da: it's a read only structure and no copy was made. In our world,
> we could do better:
>
> private final ImmutableLiteList magicValues;
> {
>LiteList buildValues = new LiteList();
>buildValues.add("able");
>buildValues.add("baker");
>buildValues.add("charlie");
>magicValues = buildValues.immutableView(); // more equivalent of
> cast()
> }
>

Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Ray Ryan
My argument is that one is necessary and sufficient. Two is kind of
pointless if you have achieved one, and maybe even counterproductive.

On Mon, Mar 22, 2010 at 2:32 PM, Joel Webber  wrote:

> I think we're talking about two different things here. Rodrigo's (valid)
> point is that implementing immutability sanely early on is a good idea. And
> this implementation is pretty much analogous to the one you describe from
> Cocoa.
>
> The question at hand is whether it makes sense to get an immutable
> collection from a mutable one, with no copies. There are two ways to do
> this:
> 1. Create an immutable "view" of a mutable list, but with no guarantees
> that the list won't be mutated by the original owner later.
> 2. "Freeze" a mutable list into an immutable view of said list, making the
> former "runtime immutable".
>
> (1) solves the problem of a class giving access to one of its internal
> collections without having to guard against external mutation. (2) can be
> used to replicate the "builder" pattern.
>
> I don't have a strong opinion about (2) myself, but (1) is pretty damned
> important, because it's the source of innumerable stupid defensive copies in
> JRE code. The provider of such an interface would just have to be very clear
> about whether the "immutable" list might be modified later (because it's a
> view on a mutable one).
>
> On Mon, Mar 22, 2010 at 5:23 PM, Ray Ryan  wrote:
>
>> I think you're missing my point. An object is immutable if there exists no
>> api to mutate it. That should be enough.
>>
>> Let me put it another way. It's lame that the JRE achieves immutability by
>> turning mutate methods into runtime errors. It will be equally lame of us to
>> do the same, especially since we can't enforce it at production time. It
>> would be much better to provide an api such that there is not even possible
>> to compile the mutate call (without cheating with casts, but then you know
>> you're being bad).
>>
>> The Cocoa approach to this is to have interfaces like NSArray be
>> immutable, and then have NSMutableArray extend NSArray. If we're going to
>> roll our own collection classes, it seems to me we could do the same: e.g.
>> LiteImmutableList and List extends LiteImmutableList.
>>
>> rjrjr
>>
>>
>> On Mon, Mar 22, 2010 at 2:12 PM, Rodrigo Chandia wrote:
>>
>>> I like the *concept* of immutability being introduced early in the
>>> development. The initial implementation may be limiting for some use cases,
>>> but I believe it is a useful concept to expand on. If specific needs require
>>> simultaneous mutable and immutable access we can provide implementations to
>>> address that problem (copy on write semantics, for example).
>>>
>>> 2010/3/22 Ray Ryan 
>>>
>>> I guess I'm overstating my opposition. It's not really dangerous, but it
 just doesn't seem useful. Just by existing I think it'll promote confusion
 and perhaps bad habits. Why bother?

 I think the 90% use case is for something like the following (writing in
 JRE terms here):

 private final List magicValues;
 {
List buildValues = new ArrayList();
buildValues.add("able");
buildValues.add("baker");
buildValues.add("charlie");
magicValues = Collections.unmodifiableList(buildValues);
 }

 Ta da: it's a read only structure and no copy was made. In our world, we
 could do better:

 private final ImmutableLiteList magicValues;
 {
LiteList buildValues = new LiteList();
buildValues.add("able");
buildValues.add("baker");
buildValues.add("charlie");
magicValues = buildValues.immutableView(); // more equivalent of
 cast()
 }

 The user never thinks in terms of freezing, just cutting off access. No
 extra dev mode mechanism to maintain, and basically the same idiom already
 in use in Java.

>>>
>>>
>>  --
>> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>
>> To unsubscribe from this group, send email to
>> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
>> this email with the words "REMOVE ME" as the subject.
>>
>
>  --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Joel Webber
I think we're talking about two different things here. Rodrigo's (valid)
point is that implementing immutability sanely early on is a good idea. And
this implementation is pretty much analogous to the one you describe from
Cocoa.

The question at hand is whether it makes sense to get an immutable
collection from a mutable one, with no copies. There are two ways to do
this:
1. Create an immutable "view" of a mutable list, but with no guarantees that
the list won't be mutated by the original owner later.
2. "Freeze" a mutable list into an immutable view of said list, making the
former "runtime immutable".

(1) solves the problem of a class giving access to one of its internal
collections without having to guard against external mutation. (2) can be
used to replicate the "builder" pattern.

I don't have a strong opinion about (2) myself, but (1) is pretty damned
important, because it's the source of innumerable stupid defensive copies in
JRE code. The provider of such an interface would just have to be very clear
about whether the "immutable" list might be modified later (because it's a
view on a mutable one).

On Mon, Mar 22, 2010 at 5:23 PM, Ray Ryan  wrote:

> I think you're missing my point. An object is immutable if there exists no
> api to mutate it. That should be enough.
>
> Let me put it another way. It's lame that the JRE achieves immutability by
> turning mutate methods into runtime errors. It will be equally lame of us to
> do the same, especially since we can't enforce it at production time. It
> would be much better to provide an api such that there is not even possible
> to compile the mutate call (without cheating with casts, but then you know
> you're being bad).
>
> The Cocoa approach to this is to have interfaces like NSArray be immutable,
> and then have NSMutableArray extend NSArray. If we're going to roll our own
> collection classes, it seems to me we could do the same: e.g.
> LiteImmutableList and List extends LiteImmutableList.
>
> rjrjr
>
>
> On Mon, Mar 22, 2010 at 2:12 PM, Rodrigo Chandia wrote:
>
>> I like the *concept* of immutability being introduced early in the
>> development. The initial implementation may be limiting for some use cases,
>> but I believe it is a useful concept to expand on. If specific needs require
>> simultaneous mutable and immutable access we can provide implementations to
>> address that problem (copy on write semantics, for example).
>>
>> 2010/3/22 Ray Ryan 
>>
>> I guess I'm overstating my opposition. It's not really dangerous, but it
>>> just doesn't seem useful. Just by existing I think it'll promote confusion
>>> and perhaps bad habits. Why bother?
>>>
>>> I think the 90% use case is for something like the following (writing in
>>> JRE terms here):
>>>
>>> private final List magicValues;
>>> {
>>>List buildValues = new ArrayList();
>>>buildValues.add("able");
>>>buildValues.add("baker");
>>>buildValues.add("charlie");
>>>magicValues = Collections.unmodifiableList(buildValues);
>>> }
>>>
>>> Ta da: it's a read only structure and no copy was made. In our world, we
>>> could do better:
>>>
>>> private final ImmutableLiteList magicValues;
>>> {
>>>LiteList buildValues = new LiteList();
>>>buildValues.add("able");
>>>buildValues.add("baker");
>>>buildValues.add("charlie");
>>>magicValues = buildValues.immutableView(); // more equivalent of
>>> cast()
>>> }
>>>
>>> The user never thinks in terms of freezing, just cutting off access. No
>>> extra dev mode mechanism to maintain, and basically the same idiom already
>>> in use in Java.
>>>
>>
>>
>  --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Ray Ryan
I think you're missing my point. An object is immutable if there exists no
api to mutate it. That should be enough.

Let me put it another way. It's lame that the JRE achieves immutability by
turning mutate methods into runtime errors. It will be equally lame of us to
do the same, especially since we can't enforce it at production time. It
would be much better to provide an api such that there is not even possible
to compile the mutate call (without cheating with casts, but then you know
you're being bad).

The Cocoa approach to this is to have interfaces like NSArray be immutable,
and then have NSMutableArray extend NSArray. If we're going to roll our own
collection classes, it seems to me we could do the same: e.g.
LiteImmutableList and List extends LiteImmutableList.

rjrjr

On Mon, Mar 22, 2010 at 2:12 PM, Rodrigo Chandia wrote:

> I like the *concept* of immutability being introduced early in the
> development. The initial implementation may be limiting for some use cases,
> but I believe it is a useful concept to expand on. If specific needs require
> simultaneous mutable and immutable access we can provide implementations to
> address that problem (copy on write semantics, for example).
>
> 2010/3/22 Ray Ryan 
>
> I guess I'm overstating my opposition. It's not really dangerous, but it
>> just doesn't seem useful. Just by existing I think it'll promote confusion
>> and perhaps bad habits. Why bother?
>>
>> I think the 90% use case is for something like the following (writing in
>> JRE terms here):
>>
>> private final List magicValues;
>> {
>>List buildValues = new ArrayList();
>>buildValues.add("able");
>>buildValues.add("baker");
>>buildValues.add("charlie");
>>magicValues = Collections.unmodifiableList(buildValues);
>> }
>>
>> Ta da: it's a read only structure and no copy was made. In our world, we
>> could do better:
>>
>> private final ImmutableLiteList magicValues;
>> {
>>LiteList buildValues = new LiteList();
>>buildValues.add("able");
>>buildValues.add("baker");
>>buildValues.add("charlie");
>>magicValues = buildValues.immutableView(); // more equivalent of cast()
>> }
>>
>> The user never thinks in terms of freezing, just cutting off access. No
>> extra dev mode mechanism to maintain, and basically the same idiom already
>> in use in Java.
>>
>
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Rodrigo Chandia
I like the *concept* of immutability being introduced early in the
development. The initial implementation may be limiting for some use cases,
but I believe it is a useful concept to expand on. If specific needs require
simultaneous mutable and immutable access we can provide implementations to
address that problem (copy on write semantics, for example).

2010/3/22 Ray Ryan 

> I guess I'm overstating my opposition. It's not really dangerous, but it
> just doesn't seem useful. Just by existing I think it'll promote confusion
> and perhaps bad habits. Why bother?
>
> I think the 90% use case is for something like the following (writing in
> JRE terms here):
>
> private final List magicValues;
> {
>List buildValues = new ArrayList();
>buildValues.add("able");
>buildValues.add("baker");
>buildValues.add("charlie");
>magicValues = Collections.unmodifiableList(buildValues);
> }
>
> Ta da: it's a read only structure and no copy was made. In our world, we
> could do better:
>
> private final ImmutableLiteList magicValues;
> {
>LiteList buildValues = new LiteList();
>buildValues.add("able");
>buildValues.add("baker");
>buildValues.add("charlie");
>magicValues = buildValues.immutableView(); // more equivalent of cast()
> }
>
> The user never thinks in terms of freezing, just cutting off access. No
> extra dev mode mechanism to maintain, and basically the same idiom already
> in use in Java.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Ray Ryan
I guess I'm overstating my opposition. It's not really dangerous, but it
just doesn't seem useful. Just by existing I think it'll promote confusion
and perhaps bad habits. Why bother?

I think the 90% use case is for something like the following (writing in JRE
terms here):

private final List magicValues;
{
   List buildValues = new ArrayList();
   buildValues.add("able");
   buildValues.add("baker");
   buildValues.add("charlie");
   magicValues = Collections.unmodifiableList(buildValues);
}

Ta da: it's a read only structure and no copy was made. In our world, we
could do better:

private final ImmutableLiteList magicValues;
{
   LiteList buildValues = new LiteList();
   buildValues.add("able");
   buildValues.add("baker");
   buildValues.add("charlie");
   magicValues = buildValues.immutableView(); // more equivalent of cast()
}

The user never thinks in terms of freezing, just cutting off access. No
extra dev mode mechanism to maintain, and basically the same idiom already
in use in Java.

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread John Tamplin
On Mon, Mar 22, 2010 at 3:18 PM, Bruce Johnson  wrote:

> @John: I totally agree that's a risk, but then again, the situation you
> describe would arguably be a bug anyway -- or at least I'd call it
> under-specified. Indeed, I hope that in people's paranoia to avoid those
> situations, that they are more thoughtful about the types they hand around
> in their API, using the exactly the right semantics for the situation at
> hand.
>
> And, of course, there are always tradeoffs in design. The design of these
> classes to enable people who are coding things correctly and clearly to not
> suffer one iota of overhead for which they don't get commensurate value.
> That's same reasoning is why it will use assertions instead of exceptions.
>

How about a big red warning on the freeze() saying if you are going to call
this method you shouldn't hand out the unfrozen object to anyone else?

-- 
John A. Tamplin
Software Engineer (GWT), Google

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Freeland Abbott
First comment, I'm glad we've provoked this re-discussion, because "the
original wave" was by definition not public.

Back to your regular programming, I think the *intended *pattern is that a
given mutable thing would only be "owned" by one entity, which would
therefore have control of whether it was frozen or not.  So, the argument
probably comes down to whether there's a good use case that violates that
expectation.  The example I can think of is something like a shared bus,
where several clients might add items to the queue... but the Java idiom for
that would we to embed the mutable list into an EventBus or WorkQueue
instance anyway, and so it would be the owner, and clients would call an
addItem(foo) method on that wrapper.

The other argument, of course, is just that given rope, *somebody* will tie
a noose and stick their head into it... it's not clear we have to stop them,
though it'd be nice to.  But if we don't stop it, we'd like it to be easy
for them to figure out what they did wrong and what to do about it!



On Mon, Mar 22, 2010 at 3:18 PM, Bruce Johnson  wrote:

> @John: I totally agree that's a risk, but then again, the situation you
> describe would arguably be a bug anyway -- or at least I'd call it
> under-specified. Indeed, I hope that in people's paranoia to avoid those
> situations, that they are more thoughtful about the types they hand around
> in their API, using the exactly the right semantics for the situation at
> hand.
>
> And, of course, there are always tradeoffs in design. The design of these
> classes to enable people who are coding things correctly and clearly to not
> suffer one iota of overhead for which they don't get commensurate value.
> That's same reasoning is why it will use assertions instead of exceptions.
>
> On Mon, Mar 22, 2010 at 3:05 PM, John Tamplin  wrote:
>
>> On Mon, Mar 22, 2010 at 2:19 PM, Freeland Abbott wrote:
>>
>>> The claim is that you make an ImmutableFoo by "freezing" a MutableFoo,
>>> after which the invariant is that no client will change that collection.  It
>>> isn't a copy, it's a freeze of the thing, so the flag blocks you from
>>> changing via the original MutableFoo handle.
>>>
>>> Contrast with vanilla Foo, which doesn't have an API for you to change
>>> it, but is allowed to change by some other bit (e.g. I have a MutableFoo,
>>> and return to you casting to Foo... I can change it, you can't).
>>>
>>> If you want a copy, copy it yourself (and pay the copy cost explicitly,
>>> then freeze one, and you can go on changing the other).  Bruce wants to run
>>> pretty close to the wire, so if you mess it up assertions will tell you
>>> about the error, but optimized it doesn't, and YMMV w.r.t. the effects.
>>>  Since devmode is always asssertions-on, the expectation is you'd find the
>>> error soon.
>>>
>>
>> There was a long discussion about this very point on the original wave of
>> the design.
>>
>> The problem I have with it is a client may have been given a MutableFoo
>> and then at some point later that same object gets frozen and it is no
>> longer mutable, and MutableFoo operations will fail on that object without
>> any notification of the client that got the MutableFoo that its contract had
>> changed.
>>
>> You can work around it with conventions, but it seems
>>
>> --
>> John A. Tamplin
>> Software Engineer (GWT), Google
>>
>
>  --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>
> To unsubscribe from this group, send email to
> google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to
> this email with the words "REMOVE ME" as the subject.
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Bruce Johnson
@John: I totally agree that's a risk, but then again, the situation you
describe would arguably be a bug anyway -- or at least I'd call it
under-specified. Indeed, I hope that in people's paranoia to avoid those
situations, that they are more thoughtful about the types they hand around
in their API, using the exactly the right semantics for the situation at
hand.

And, of course, there are always tradeoffs in design. The design of these
classes to enable people who are coding things correctly and clearly to not
suffer one iota of overhead for which they don't get commensurate value.
That's same reasoning is why it will use assertions instead of exceptions.

On Mon, Mar 22, 2010 at 3:05 PM, John Tamplin  wrote:

> On Mon, Mar 22, 2010 at 2:19 PM, Freeland Abbott wrote:
>
>> The claim is that you make an ImmutableFoo by "freezing" a MutableFoo,
>> after which the invariant is that no client will change that collection.  It
>> isn't a copy, it's a freeze of the thing, so the flag blocks you from
>> changing via the original MutableFoo handle.
>>
>> Contrast with vanilla Foo, which doesn't have an API for you to change it,
>> but is allowed to change by some other bit (e.g. I have a MutableFoo, and
>> return to you casting to Foo... I can change it, you can't).
>>
>> If you want a copy, copy it yourself (and pay the copy cost explicitly,
>> then freeze one, and you can go on changing the other).  Bruce wants to run
>> pretty close to the wire, so if you mess it up assertions will tell you
>> about the error, but optimized it doesn't, and YMMV w.r.t. the effects.
>>  Since devmode is always asssertions-on, the expectation is you'd find the
>> error soon.
>>
>
> There was a long discussion about this very point on the original wave of
> the design.
>
> The problem I have with it is a client may have been given a MutableFoo and
> then at some point later that same object gets frozen and it is no longer
> mutable, and MutableFoo operations will fail on that object without any
> notification of the client that got the MutableFoo that its contract had
> changed.
>
> You can work around it with conventions, but it seems
>
> --
> John A. Tamplin
> Software Engineer (GWT), Google
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Bruce Johnson
Here's how freeze() got introduced.

You need to be able to have ImmutableArray without any mutators, and you
need to be able to create them, thus you need a builder. A very frequent
pattern will be to build up an array with a builder (the hypothetical
ImmutableArrayBuilder) and then want to get an ImmutableArray from it. So,
you'd often write this:

== BEGIN SNIPPET ==
ImmutableArrayBuilder b = new ImmutableArrayBuilder();
b.add(...);
...
ImmutableArray ia = b.build();

// throw away b
== END SNIPPET ==

A few observations about the above inevitable code snippet:
1) ImmutableArrayBuilder would have an API that's almost the same as
MutableArray, so why make it a separate class?
2) The builder pattern typically allows multiple objects to be built from
the same builder, but it's easy to imagine that typical uses won't actually
want to reuse builder instances, thus we're paying for twice the number of
object allocations (1 builder + 1 product) for the common case.

Why solve it with freeze()? This design makes MutableArray into a builder of
ImmutableArrays that is extremely efficient (since it will be implemented as
"return this" and will use JSO cross-casting), thus avoiding the wasteful
builder allocation problem. Secondly, the "freeze()" semantics don't open up
the notion that a single MutableArray could be used as a reusable factory --
unlike build(), freeze() makes it obvious that you can't mess with the
builder anymore.

Hope that makes sense. As Freeland said, this design is based on the idea of
maximum performance, minimum size, and a set of types that allows
applications to avoid expensive allocations and copying. We need a rich
enough set of types that, whatever the circumstances, you're never more than
cheap (hopefully O(1)-time) operation away from having an object that
satisfies the need. For example, we want to encourage handing out aliases to
private fields (making it safe to do so by providing a read-only handle in
the form of the root type).

On Mon, Mar 22, 2010 at 2:06 PM, Ray Ryan  wrote:

> Can you outline a use case? I don't get it. My argument isn't with
> isFrozen, it's with the freezing feature per se. I can't see a reasonable
> use for it.
>
>
> On Mon, Mar 22, 2010 at 11:03 AM, Rodrigo Chandia wrote:
>
>> isFrozen allows assertions on the status of a mutable collection. During
>> normal use (assertions disabled), there should be no need to call isFrozen.
>> Moreover, using isFrozen outside of an assertion, or while assertions are
>> disabled, is not guaranteed to work at all. The intention is to avoid having
>> to pay a runtime penalty and discourage defensive programming.
>>
>> Related to the review, it was not my intention to introduce isFrozen just
>> yet (but it slipped through, sorry). isFrozen is a construct that only makes
>> sense when when Immutable classes are introduced, along with assertions and
>> tests relevant to immutability. At this point I wanted to concentrate on the
>> Mutable and the parent Read-only classes.
>>
>> Is isFrozen still a bad idea when used for assertions only?
>>
>>
>> 2010/3/22 
>>
>> Can someone explain why isFrozen is a good idea? It sounds really,
>>> really bad to me.
>>>
>>>
>>> http://gwt-code-reviews.appspot.com/232801/show
>>>
>>
>>
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


Re: [gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread John Tamplin
On Mon, Mar 22, 2010 at 2:19 PM, Freeland Abbott  wrote:

> The claim is that you make an ImmutableFoo by "freezing" a MutableFoo,
> after which the invariant is that no client will change that collection.  It
> isn't a copy, it's a freeze of the thing, so the flag blocks you from
> changing via the original MutableFoo handle.
>
> Contrast with vanilla Foo, which doesn't have an API for you to change it,
> but is allowed to change by some other bit (e.g. I have a MutableFoo, and
> return to you casting to Foo... I can change it, you can't).
>
> If you want a copy, copy it yourself (and pay the copy cost explicitly,
> then freeze one, and you can go on changing the other).  Bruce wants to run
> pretty close to the wire, so if you mess it up assertions will tell you
> about the error, but optimized it doesn't, and YMMV w.r.t. the effects.
>  Since devmode is always asssertions-on, the expectation is you'd find the
> error soon.
>

There was a long discussion about this very point on the original wave of
the design.

The problem I have with it is a client may have been given a MutableFoo and
then at some point later that same object gets frozen and it is no longer
mutable, and MutableFoo operations will fail on that object without any
notification of the client that got the MutableFoo that its contract had
changed.

You can work around it with conventions, but it seems

-- 
John A. Tamplin
Software Engineer (GWT), Google

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread rice


http://gwt-code-reviews.appspot.com/232801/diff/3001/4002
File bikeshed/src/com/google/gwt/collections/Assertions.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4002#newcode24
bikeshed/src/com/google/gwt/collections/Assertions.java:24: assert
(index >= 0 && index < maxExclusive) : msgIndexOutOfRange(index,
minInclusive,
minInclusive is never checked here

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Freeland Abbott
The claim is that you make an ImmutableFoo by "freezing" a MutableFoo, after
which the invariant is that no client will change that collection.  It isn't
a copy, it's a freeze of the thing, so the flag blocks you from changing via
the original MutableFoo handle.

Contrast with vanilla Foo, which doesn't have an API for you to change it,
but is allowed to change by some other bit (e.g. I have a MutableFoo, and
return to you casting to Foo... I can change it, you can't).

If you want a copy, copy it yourself (and pay the copy cost explicitly, then
freeze one, and you can go on changing the other).  Bruce wants to run
pretty close to the wire, so if you mess it up assertions will tell you
about the error, but optimized it doesn't, and YMMV w.r.t. the effects.
 Since devmode is always asssertions-on, the expectation is you'd find the
error soon.




On Mon, Mar 22, 2010 at 2:06 PM, Ray Ryan  wrote:

> Can you outline a use case? I don't get it. My argument isn't with
> isFrozen, it's with the freezing feature per se. I can't see a reasonable
> use for it.
>
>
> On Mon, Mar 22, 2010 at 11:03 AM, Rodrigo Chandia wrote:
>
>> isFrozen allows assertions on the status of a mutable collection. During
>> normal use (assertions disabled), there should be no need to call isFrozen.
>> Moreover, using isFrozen outside of an assertion, or while assertions are
>> disabled, is not guaranteed to work at all. The intention is to avoid having
>> to pay a runtime penalty and discourage defensive programming.
>>
>> Related to the review, it was not my intention to introduce isFrozen just
>> yet (but it slipped through, sorry). isFrozen is a construct that only makes
>> sense when when Immutable classes are introduced, along with assertions and
>> tests relevant to immutability. At this point I wanted to concentrate on the
>> Mutable and the parent Read-only classes.
>>
>> Is isFrozen still a bad idea when used for assertions only?
>>
>>
>> 2010/3/22 
>>
>> Can someone explain why isFrozen is a good idea? It sounds really,
>>> really bad to me.
>>>
>>>
>>> http://gwt-code-reviews.appspot.com/232801/show
>>>
>>
>>
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Ray Ryan
Can you outline a use case? I don't get it. My argument isn't with isFrozen,
it's with the freezing feature per se. I can't see a reasonable use for it.

On Mon, Mar 22, 2010 at 11:03 AM, Rodrigo Chandia wrote:

> isFrozen allows assertions on the status of a mutable collection. During
> normal use (assertions disabled), there should be no need to call isFrozen.
> Moreover, using isFrozen outside of an assertion, or while assertions are
> disabled, is not guaranteed to work at all. The intention is to avoid having
> to pay a runtime penalty and discourage defensive programming.
>
> Related to the review, it was not my intention to introduce isFrozen just
> yet (but it slipped through, sorry). isFrozen is a construct that only makes
> sense when when Immutable classes are introduced, along with assertions and
> tests relevant to immutability. At this point I wanted to concentrate on the
> Mutable and the parent Read-only classes.
>
> Is isFrozen still a bad idea when used for assertions only?
>
>
> 2010/3/22 
>
> Can someone explain why isFrozen is a good idea? It sounds really,
>> really bad to me.
>>
>>
>> http://gwt-code-reviews.appspot.com/232801/show
>>
>
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread Rodrigo Chandia
isFrozen allows assertions on the status of a mutable collection. During
normal use (assertions disabled), there should be no need to call isFrozen.
Moreover, using isFrozen outside of an assertion, or while assertions are
disabled, is not guaranteed to work at all. The intention is to avoid having
to pay a runtime penalty and discourage defensive programming.

Related to the review, it was not my intention to introduce isFrozen just
yet (but it slipped through, sorry). isFrozen is a construct that only makes
sense when when Immutable classes are introduced, along with assertions and
tests relevant to immutability. At this point I wanted to concentrate on the
Mutable and the parent Read-only classes.

Is isFrozen still a bad idea when used for assertions only?


2010/3/22 

> Can someone explain why isFrozen is a good idea? It sounds really,
> really bad to me.
>
>
> http://gwt-code-reviews.appspot.com/232801/show
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this 
email with the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread rjrjr

Can someone explain why isFrozen is a good idea? It sounds really,
really bad to me.

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread fabbott

Comments as noted; also, what semantics do we want for equality?
(Identity is certainly cheapest, and probably good, just wanted to make
an explicit choice there).

I think you're buggy if your first element is null, and I'm not entirely
sure the singleElem special case is worth the time-cost of your decision
branches.

I think you will be buggy, once you add immutability, if you don't check
isfrozen.







http://gwt-code-reviews.appspot.com/232801/diff/3001/4002
File bikeshed/src/com/google/gwt/collections/Assertions.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4002#newcode37
bikeshed/src/com/google/gwt/collections/Assertions.java:37: static
String msgShouldNotBeNull() {
I'm not entirely sure these are useful (vice just using the constants
in-line in the sole call sites), but okay...

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007
File bikeshed/src/com/google/gwt/collections/MutableArray.java (right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode23
bikeshed/src/com/google/gwt/collections/MutableArray.java:23: // Used
when there is exactly one element in progress
"in progress" doesn't mean much for me here.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode38
bikeshed/src/com/google/gwt/collections/MutableArray.java:38:
@SuppressWarnings("unchecked")
what's the unchecked warning from?

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode56
bikeshed/src/com/google/gwt/collections/MutableArray.java:56: } else if
(singleElem != null) {
two questions: first, is the whole singleElem special case actually
gaining anything here?  second, am I allowed to have an array of size 1
containing only null?  (And if not, why not, and how do I know?)

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode75
bikeshed/src/com/google/gwt/collections/MutableArray.java:75:
I think you need a check here (and remove and set) against isfrozen?
(Or, if immutability is a copy op, you don't need isfrozen, one or the
other...)

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode79
bikeshed/src/com/google/gwt/collections/MutableArray.java:79: //
TODO(bruce): grow smartly
not likely to be bruce.  (Not sure we attribute our todo's generally,
anyway.)

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode92
bikeshed/src/com/google/gwt/collections/MutableArray.java:92: singleElem
= elem;
you lose if elem==null

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode110
bikeshed/src/com/google/gwt/collections/MutableArray.java:110: // Bad!
But only happens if index is bad and assertions are off
I know bruce was imagining minimal safety overhead, but I'd like to see
an assertion here, just in case we're someday wrong/buggy about the
precondition assertions.  I can live with assertions-only correctness
checking, but get queasy about deliberate holes.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4007#newcode131
bikeshed/src/com/google/gwt/collections/MutableArray.java:131: // Bad!
But only happens if index is bad and assertions are off
as above, wrt assertions for checking.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008
File bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java
(right):

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode29
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:29: throw
new IllegalStateException("This test case requires assertions to be
enabled");
um.  I dunno that this should be true, or if true I think we might want
it to pass (but be a no-op) rather than fail when run without 'em.

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode108
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:108:
b.add(null);
yeah, try doing this (add of null) as your first item. ;-)

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode126
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:126:
b.set(3, "eclair");
check result?

http://gwt-code-reviews.appspot.com/232801/diff/3001/4008#newcode177
bikeshed/test/com/google/gwt/collections/ObjectArrayTest.java:177:
fail("That should have been okay");
can you check the assertion message, here and elsewhere, to be sure it's
the one we want?  I remember seeing some issues with junit 4 where
assertions could cause problems (e.g. the junit assertions and your SUT
assertions co-mingled).

Also, again, I'd rather you checked assertion state and decided what to
expect (or short-circuited the test) then, so the test will "work" in
either mode.

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.


[gwt-contrib] Re: Array implementation for Lightweight Collections. Pure Java implementation only. (issue232801)

2010-03-22 Thread rchandia

http://gwt-code-reviews.appspot.com/232801/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.