Daniel L. Rall said: > Why are you catching Exception? consistency. it's what i've been catching elsewhere in the class.
> Nothing in that block throws Exception. following that argument, nothing in that block throws RuntimeException either. in fact, i've been in java's Date/Calendar/Format classes enough to know that the only thing we could really get here is IllegalArgumentExceptions. > If later a checked exception is introduced into the main try block there, a > developer may not realize it due to this overzealous exception supression. it's wasn't overzealous exception suppression, but overzealous consistency. :) > I'd much prefer that you revert that to catch RuntimeException -- its use was > a conscious choice which preserves the semantics of the method. Complete > supression of exceptions (unchecked or otherwise) must be done with care. :) true enough. i guess i just don't see it as a noteworthy issue here. subclasses can't override anything inside that block. the only thing that block will throw is IAE. i have weird inclinations to make code consistent. changing the one place was easier than changing all places we suppress exceptions in DateTool. > Returning null explicitly also changes the semantics of the method. > Previously, you could've created a DateFormat instance, but not filled in its > time zone information, and still had something to return even if an exception > was thrown. Personally, I much prefer the return of null when an error > occurs, but a change in semantics demands more JavaDoc and/or a meaningful > change log message in theory, i acknowledge what you're saying here makes sense, but in this instance, it doesn't actually change anything. you couldn't make setTimeZone() throw an exception if you tried. (look through the source of Calendar and DateFormat if you don't believe me) that said, i agree that as long as we're suppressing IAEs here, i would prefer returning null to returning a half-successful result. nothing has frustrated me more in my programming years than when something "half-works" and doesn't warn me about it. but, the bottom line is that this does not actually change behavior and this method is new (within a few weeks). i see no reason it would demand notice of this in javadoc. calling it "trivial" in the commit message may not suit you, but AFAI am concerned, that's the truth of it. this method will function no differently as is than as you had it (or--arguably--even as i first committed it). that said, i have more important things to do than argue this one method. feel free to change it however makes you comfortable. as long as it functions as it does now (semantics aside), i'll leave it be. :) Nathan Bubna [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
