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/SSLIOSessio > > n.java > > b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSessio > > n.java > > index 1b350ab..7c54db3 100644 > > --- > > a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSessio > > n.java > > +++ > > b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSessio > > 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. 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. Gary > Oleg > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
