On Thu, 2018-11-22 at 08:47 -0700, Gary Gregory wrote: > On Thu, Nov 22, 2018 at 1:57 AM Oleg Kalnichevski <[email protected]> > wrote: > > > On Thu, 2018-11-22 at 01:24 +0000, [email protected] wrote: > > > Repository: httpcomponents-core > > > Updated Branches: > > > refs/heads/master adcaa9ff0 -> a91a9ca97 > > > > > > > > > Refactor to use state getter methods instead of relying on the > > > relationship between constant values. > > > > > > Project: > > > http://git-wip-us.apache.org/repos/asf/httpcomponents-core/repo > > > Commit: > > > > > > > http://git-wip-us.apache.org/repos/asf/httpcomponents-core/commit/a91a9ca9 > > > Tree: > > > http://git-wip-us.apache.org/repos/asf/httpcomponents-core/tree/a91a9ca9 > > > Diff: > > > http://git-wip-us.apache.org/repos/asf/httpcomponents-core/diff/a91a9ca9 > > > > > > Branch: refs/heads/master > > > Commit: a91a9ca971d0ef0dd5d5b6fced9a14efcd2be50c > > > Parents: adcaa9f > > > Author: Gary Gregory <[email protected]> > > > Authored: Wed Nov 21 18:24:35 2018 -0700 > > > Committer: Gary Gregory <[email protected]> > > > Committed: Wed Nov 21 18:24:35 2018 -0700 > > > > > > --------------------------------------------------------------- > > > ---- > > > --- > > > .../hc/core5/reactor/ssl/SSLIOSession.java | 36 > > > +++++++++++++ > > > ------- > > > 1 file changed, 24 insertions(+), 12 deletions(-) > > > --------------------------------------------------------------- > > > ---- > > > --- > > > > > > > > > > > > > http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/a91a9ca9/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java > > > --------------------------------------------------------------- > > > ---- > > > --- > > > diff --git > > > a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSe > > > ssio > > > n.java > > > b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSe > > > ssio > > > n.java > > > index 1b350ab..7c54db3 100644 > > > --- > > > a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSe > > > ssio > > > n.java > > > +++ > > > b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSe > > > ssio > > > n.java > > > @@ -219,7 +219,7 @@ public class SSLIOSession implements > > > IOSession { > > > Asserts.check(!this.initialized, "SSL I/O session > > > already > > > initialized"); > > > this.session.lock().lock(); > > > try { > > > - if (this.status >= IOSession.CLOSING) { > > > + if (isStatusClosing() || isStatusClosed()) { > > > > > > > Gary, > > > > Why did you have to do that? What was the point? How exactly is > > that > > better? Why do we have to have two comparison operations where > > exactly > > one would perfectly suffice? > > > > Hi Oleg, > > This is exactly why enums exists IMO; an enum shows *the intent of > the > design* in the most clear and precise manner: Here are the values > allowed, > and the *only* values allowed, for this domain. > > So here and in general, I feel it is better to use an enum to define > a > known set of values instead of integers. > > I'm glad you chose the expression "this.status >= IOSession.CLOSING" > which > is also IMO mind boggling: Performing arithmetic on states makes the > code > harder to grok and maintain; and we all know the 80/20 rule on that. >
Who exactly decreed that, I wonder? > It's also easier to introduce bugs when integers are used. If Java > had no > enumerations, then we'd have to express states using integers, bit > fields, > or some other mean, but we _do_ have enumerations which is a great > tool for > abstracting such concepts. > I do not have a problem with enums as such. I have problem with people making changes unilaterally based on their personal, very biased perception of what they see as a better style. 'this.status >= IOSession.CLOSING' means any status past CLOSING. This meaning and intent is completely lost with your changes. Please make any such changes on a branch and at least give me a chance to object. Oleg --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
