On Thu, May 14, 2020 at 3:51 PM Alex Herbert <alex.d.herb...@gmail.com> wrote:
> > > > On 14 May 2020, at 18:03, Gary Gregory <garydgreg...@gmail.com> wrote: > > > > Hi All, > > > > The addition of org.apache.commons.codec.binary.BaseNCodec.strictDecoding > > is not quite right IMO, the ivar should be final with some new ctors. ALL > > other ivars here are final and the class is documented as thread-safe. > I'll > > proceed unless someone makes a case for jumping through the extra hoops > for > > make this setting mutable AND thread-safe. > > The boolean is only used once per decode. Thus it is not thread-unsafe. It > acts like a late binding behaviour flag. Thus if it is changed half way > through decoding the effect is still consistent decoding, but maybe not > what you expected. For example if one thread checks it is strict decoding > and then starts decoding and meanwhile another thread with the same > instance updates the property to be non-strict the first thread would get a > possibly incorrect behaviour. I do not see a way around this as long as the > strict decoding flag is mutable. > > When this property was introduced we had a discussion on whether to use a > property or overloaded constructor. There are already a lot of > constructors. Base64 has 5. So to avoid deciding which constructors to add > I added this as a property. A change to remove the property setter would > have to target: > > BaseNCodec > BCodec > BaseNCodecInputStream > BaseNCodecOutputStream > > Which all have the property. > > If we are to maintain thread-safe and thread consistent behaviour then the > flag should be final. An alternative is some work behind the > isStrictDecoding property to fix the value on the first read of the field. > So you can use the setStrictDecoding setter to update the property only if > the decoder has never been used. Otherwise the value passed to the setter > is ignored and the codec always functions the same as it did on first use. > This seems worse to implement and document than just having some more > constructors and a final flag. > > So +1 to more constructors and removing the setter. > I updated git master. Note the change from a boolean to the new enum CodecPolicy{ LENIENT, STRICT } which we can reuse elsewhere instead of yet another boolean toggle. Gary > > Alex > > > > > > Gary > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >