RE: [lang][PATCH] ArrayUtil.subarray(Object, startIndex, endIndex) implementatio

["Ash .." <eq...@hotmail.com>]

Some comments:
1) You should try to match the style of the surrounding code. For your
submission this means spaces after if and for, brackets around the if
statements, and brackets at the end of lines. In many cases I just use
eclipse to reformat new code, but the missing if statement brackets would
require manual intervention.

[Ash]
I did try to match the formatting, but some of the differences in the format
seemed to have simply missed me. I'll ensure 100% conformance next 
submission onwards.



2) The array copy should be done using System.arraycopy, unless you can
demonstrate the this is faster.

[Ash]
I can't. So, will use System.arraycopy()



3) The edge case of startIndex == endIndex is not handled to return
EMPTY_OBJECT_ARRAY

[Ash]
I left this as a consequence of the final algorithm. I will treat it as a 
special case and insert the check.


4) The parameter names would be better as startIndexInclusive and
endIndexExclusive.

[Ash]
In my understanding, when something spans two indexes, typically, the start 
index is
inclusive and the end index exclusive. Examples in standard Java API are 
String.substring()
and Arrays.sort(). I would thus propose that the param names be startIndex 
and endIndex with
the javadoc clearly specifying the ?clusiveness.



5) The startIndex comment is innacurate re overvalue

[Ash]
In fact, it's downright wrong! Mea culpa.


6) I think that the startIndex to 0 and endIndex to end tests should
probably go before the EMPTY tests.

OK.


7) The submission was via a whole file, rather than a patch. See
http://jakarta.apache.org/commons/patches.html

[Ash]
Sorry for my ignorance about patches. Will get it ready soon.


8) Tests look good, although I would like to see some
assertSame(EMPTY_OBJECT_ARRAY, ...) to test that the constant is returned in
the relevant cases.

[Ash]
OK.


This seems like a long list, but hopefully you will learn some of our ways
through it. Please submit a patch either to the list or bugzilla if you can
;-)

[Ash]
I am learn-willing :-) Will submit the patch as soon as I can.

Thanks,
Ashwin






Stephen

----- Original Message -----
From: "Ash" <eq...@hotmail.com>
>PFE a first implementation of the proposed
>ArrayUtil.subarray()    implemented to take an Object array.
>Also encl some basic testcases.
>This is my first contribution, so I might take a while to get used to the
>some of the procedures of the group.
>Waiting for feedback and criticism.
>Thanks,
>Ashwin
>
>
>
>
>

_________________________________________________________________
On the move? Get Hotmail on your mobile phone http://www.msn.co.uk/msnmobile


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org