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
>
>

Reply via email to