Hi Sven, On Mon, 2005-05-16 at 19:15 +0200, Sven de Marothy wrote: > This reimplements the getDefaultTimeZone method of java.util.TimeZone. > It could probably be done in less code. I'm lame though. > > Now it can parse the syntax of the glibc/POSIX TZ string: > http://theory.uwinnipeg.ca/gnu/glibc/libc_303.html > > Which is good for systems which still use that old way of specifiying > timezones. > > 2005-05-16 Sven de Marothy <[EMAIL PROTECTED]> > * java/util/TimeZone (getDefaultTimeZone): Reimplemented. > (parseTime, getDateParams): New private methods.
This looks very nice. Please add a note about this to the NEWS file. Just a couple of little questions/notes/nitpicks. Just a general remark that the code does a lot of String.charAt(int) which might be inefficient. This was already the case with the old code. This code isn't that speed critical since the string to parse is in general short and only executes once. > + while (c != '+' && c != '-' && c != ',' && c != ':' > + && ! Character.isDigit(c) && c != '\0' && index < idLength); Why that explicit test for '\0'? Can that ever happen in practise? > + stdName = sysTimeZoneId.substring(0, --index); > + prevIndex = index; > + > + if (index >= idLength) > + return (TimeZone)timezones().get(stdName); Since you just did a --index is the value of prevIndex correct and can this if statement ever succeed? > + // Done yet? (Format: std offset dst offset) > + // FIXME: We don't support DST without a rule given. Should we? I don't think so. If someone made an explicit TZ string this way they probably know what they are doing. I hope. > + // FIXME: Produce a warning here? > + catch (IndexOutOfBoundsException _) Yes, I think that would be a good idea because it seems to indicate a problem in the parsing code, not a problem with the actual string. (Although it seems the next NumberFormatException will be produced when the string is wrongly formatted, so there a warning is probably not appropriate.) Do you have a list of test strings against which you tested this? Is there a way to add such tests to mauve? Cheers, Mark
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Classpath-patches mailing list [email protected] http://lists.gnu.org/mailman/listinfo/classpath-patches
