I like this solution. If my opinion is counted: +1 from me. -- Ivan Intel Middleware Products Division 2006/5/14, Tim Ellison : > I agree with your analysis. +1 from me. > > Regards, > Tim > > Stepan Mishura wrote: > > On 5/12/06, *Jimmy, Jing Lv* wrote: > > > > > > >> 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 > > > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org > > > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org > > > > > > > > ------------------------------------------------------------------------ > > > > 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: harmony-dev-unsubscribe@incubator.apache.org > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org > > -- > > Tim Ellison (t.p.ellison@gmail.com) > IBM Java technology centre, UK. > > --------------------------------------------------------------------- > Terms of use : http://incubator.apache.org/harmony/mailing.html > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org > For additional commands, e-mail: harmony-dev-help@incubator.apache.org > > --------------------------------------------------------------------- Terms of use : http://incubator.apache.org/harmony/mailing.html To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org For additional commands, e-mail: harmony-dev-help@incubator.apache.org