On Tue, 2019-12-31 at 21:55 +0100, Michael Osipov wrote:
> Am 2019-12-31 um 21:14 schrieb Oleg Kalnichevski:
> > On Tue, 2019-12-31 at 15:10 -0500, Gary Gregory wrote:
> > > This is -1 from me, as this a classic anti-pattern: You are using
> > > the
> > > enum's ordinal value for sorting :-(
> > >
> > > I am guessing Micheal will also recognize this from Effective
> > > Java's
> > > guidelines "Never derive a value associated with an enum from its
> > > ordinal;
> > > store it in an instance field instead".
> > >
> > > Guess what happens if you change the order of the enum values in
> > > the
> > > source? Everything breaks.
> > >
> > > Please revert.
> > >
> >
> > I will revert this commit as long as I can also revert your
> > previous
> > commit if that is OK with you.
>
> Let's calm down for a moment.
>
> That's a hard nut to crack:
> I just got the book out of my shelf, item 31 says don't rely on
> #ordinal() or don't use it directly.
>
> #compareTo() makes indirect use, granted. What about a comprise:
> Since this is only three value enum, used primarily internally
> (hence
> the @Internal). It is OK to rely on the physical order if AND ONLY
> if
> there is a decent comment that the order must be retained.
>
Exactly the same equally applies to ranking in the original change-set.
> But I tend to agree with Gary, if that would be a more complex,
> public
> facing enum, relying on #ordinal() would be clearly a no-go.
>
> The purpose of replacing static ints with an enum is described in
> item
> 30 to give the lib programmer full control over the type, instead of
> relying on a general purpose type like it which can have any value.
>
> @Oleg, I'd be fine if you would add a comment to that enum.
>
The logical sequence {ACTIVE, CLOSING, CLOSED} sounds pretty self-
explanatory, but I added a comment to that effect per your request
https://github.com/apache/httpcomponents-core/commit/8511b22e0776e63a66871a4bed512f289b004b51#diff-02c5f2fb7be4163f79bc0db4642ee2c6R58
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
