[m-rev.] For review: Java implementation of array library

[James Goddard]

 --- Fergus Henderson <fjh at cs.mu.OZ.AU> wrote: > On 08-Jan-2004, James Goddard
<goddardjames at yahoo.com> wrote:
> > +	if (Array0 == null) {
> > +		Array = java.lang.reflect.Array.newInstance(itemClass, Size);
> > +		for (int i = 0; i < Size; i++) {
> > +			java.lang.reflect.Array.set(Array, i, Item);
> > +		}
> 
> I notice here that if Size == 0, this will create a non-null Array
> with length zero.  That means there are two different representations
> for empty arrays, which might cause problems...
>

Good point - I didn't consider that case. Empty arrays are supposed to be null.

> > +	} else if (java.lang.reflect.Array.getLength(Array0) == Size) {
> > +		Array = Array0;
> > +	} else if (java.lang.reflect.Array.getLength(Array0) > Size) {
> > +		Array = java.lang.reflect.Array.newInstance(itemClass, Size);
> > +		for (int i = 0; i < Size; i++) {
> > +			java.lang.reflect.Array.set(Array, i,
> > +					java.lang.reflect.Array.get(Array0, i)
> > +					);
> > +		}
> > +	} else {
> > +		Array = java.lang.reflect.Array.newInstance(itemClass, Size);
> > +
> > +		int i;
> > +		for (i = 0; i < java.lang.reflect.Array.getLength(Array0); i++)
> > +		{
> > +			java.lang.reflect.Array.set(Array, i,
> > +					java.lang.reflect.Array.get(Array0, i)
> > +					);
> > +		}
> > +		for (/*i = Array0.length*/; i < Size; i++) {
> > +			java.lang.reflect.Array.set(Array, i, Item);
> > +		}
> > +	}
> > +").
> 
> The second-last case here, "getLength(Array0) > Size", is unnecessary --
> the code in the final "else" case will do the right thing for the case
> where getLength(Array0) > Size, won't it?
> 

It won't do *quite* the right thing. If Array0 is longer than Array, the loop

> > +		for (i = 0; i < java.lang.reflect.Array.getLength(Array0); i++)

will try to write past the end of Array and throw an exception.  I could merge
these two cases by changing the loop condition to

i < java.lang.reflect.Array.getLength(Array0) && i < Size

if you like. It just means there'll be an extra comparison on the inner loop,
which is a little less efficient.  I'll post a relative diff in a minute.

James

http://personals.yahoo.com.au - Yahoo! Personals
New people, new possibilities. FREE for a limited time.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------