I'd like to defer these changes to after the release. The behaviour is more than 5 years old (coming from Batik). Changing the behaviour will require testing withing Batik (and FOP). After all, this is no blocker, just cosmetics.
On 17.11.2006 18:15:22 Andreas L Delmelle wrote: > On Nov 17, 2006, at 15:55, Glen Mazza wrote: > > <snip /> > > 2.) Methods setColor(), setFont(), setPaint(), setBackground(), > > and clip() quietly just return and rely on the previous values if > > the caller provides a NULL argument. It should set the value to > > NULL instead and let the NPE occur naturally--that provides the > > quickest way for the developer to realize that he or she goofed > > up. If setFont(NULL) just relies on the previous value of the > > font, as it does now, it is much harder to track the error further > > downstream in the code. See [2]. > > Agreed. > > I'd either let the NPE occur naturally, as Glen proposes, or at least > give the user-developer an indication that the use of a null argument > is dubious... I remember a similar situation in FOP's properties > package, but there, FOP itself was the only possible caller as the > class was not part of the public API. In FOP, I remember having opted > for a solution which explicitly prevented a null from being passed in > as an argument to the PropertyParser, since it looked as if nulls > were never supposed to make it in there. > > Either nulls are acceptable as an argument --but then at least > something should happen, which is clearly not the case here-- or they > aren't. One only needs to ask the question: "Why would anyone > explicitly call those methods with a null argument?" You either set > the Color or you don't, but setting the Color to null makes very > little sense... > > If Commons cannot contain a dependency on a logging library, then > Glen's proposal is the only right way to deal with this IMO. The > caller could then decide to allow this dubious use of the method and > catch the NPE (*), but Commons itself should not allow it (or at > least: it should not take this possibility into account). > > (*) Note that it would be much better programming-style to avoid the > method from ever being called in that case, just like it is a sign of > bad style to try-and-catch an ArrayIndexOutOfBoundsException instead > of making sure beforehand that the index lies within the array's > bounds. A simple check to avoid the error is far more efficient than > waiting for an Exception to be thrown. > > > Just my 2 cents... > > Cheers, > > Andreas Jeremias Maerki --------------------------------------------------------------------- Apache XML Graphics Project URL: http://xmlgraphics.apache.org/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
