I agree with your analysis. +1 from me.
Regards,
Tim
Stepan Mishura wrote:
> On 5/12/06, *Jimmy, Jing Lv* wrote:
>
> <SNIP>
> >> In this case, though replace StringIndexOutOfBoundsException with
> >> ArrayIndexOutOfBoundsException is surely better, it seems it is
> internal
> >> implementation what cause the problem. According to the code it use
> >> String.valueof(str), which writes:
> >> try {
> >> System.arraycopy(data, start, value, 0, count);
> >> } catch (IndexOutOfBoundsException e) {
> >> throw new StringIndexOutOfBoundsException();
> >> }
> >
> >
> > IMHO this try-catch block is redandunt - the method code already
> contains
> > checks to verify that all parameters are valid:
> > if (start >= 0 && 0 <= length && length <= data.length - start) {
> > ....
> > else
> > throw new StringIndexOutOfBoundsException();
> >
>
> I believe you are right, but there may be some reasons for the author
> to write such try{...}catch(){}, perhaps he do follow RI's exception in
> the class String.
>
>
> Hi,
>
> I'd like to acquire golden ticket for updating String.java file. Please
> review my request.
>
> Description:
> Constructor: public String(char[] data, int start, int length) contains
> redundant try-catch block that should be removed:
> try {
> System.arraycopy(data, start, value, 0, count);
> } catch (IndexOutOfBoundsException e) {
> throw new StringIndexOutOfBoundsException();
> }
>
> Basis:
> According to the spec. method
> System.arraycopy(Object src, int srcPos, Object dest, int destPos, int
> length)
>
> throws IndexOutOfBoundsException in the following cases:
> 1) srcPos < 0
> 2) destPos < 0
> 3) length <0
> 4) srcPos+length > src.length
> 5) destPos+length > dest.length
>
> In our case: destPos is passed as 0 value and dest array is allocated
> as: value = new char[length]. So in 2 and 5 cases IOOBE cannot be thrown
> by arraycopy method. For other cases (1,3,4) the constructor contains
> explicit check and throws SIOOBE if passed parameters are invalid:
> if (start >= 0 && 0 <= length && length <= data.length - start) {
> ....
> } else {
> throw new StringIndexOutOfBoundsException();
>
> So System.arraycopy(...) method MUST not throw IOOBE in the the
> constructor and try-catch block is redundant in this case.
>
> Patch file is attached.
>
> Thanks,
> Stepan Mishura
> Intel Middleware Products Division
>
> ------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> <http://incubator.apache.org/harmony/mailing.html>
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail: [EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>
>
>
> ------------------------------------------------------------------------
>
> Index: modules/luni/src/main/java/java/lang/String.java
> ===================================================================
> --- modules/luni/src/main/java/java/lang/String.java (revision 405602)
> +++ modules/luni/src/main/java/java/lang/String.java (working copy)
> @@ -434,11 +434,8 @@
> offset = 0;
> value = new char[length];
> count = length;
> - try {
> - System.arraycopy(data, start, value, 0, count);
> - } catch (IndexOutOfBoundsException e) {
> - throw new StringIndexOutOfBoundsException();
> - }
> +
> + System.arraycopy(data, start, value, 0, count);
> } else
> throw new StringIndexOutOfBoundsException();
> }
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
--
Tim Ellison ([EMAIL PROTECTED])
IBM Java technology centre, UK.
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
