The first part of the condition "!this.concurrencyMessageIssued" is just
simply to make sure we only log this message once.
The second part "(tag != null) != this.concurrencyChecksEnabled" could be
changed to this:
boolean serverCCEnabled = this.concurrencyChecksEnabled;
boolean clientCCEnabled = tag != null;
if (clientCCEnabled != serverCCEnabled)
log message
The only bug I see in this method is in the actual log message.
Arg 0 describes how the server has CC configured and arg 1 describes how
the client has CC configured.
But by passing "!this.concurrencyChecksEnabled" as arg 0 the message will
actually tell you how the server is NOT configured.
And by passing "this.concurrencyChecksEnabled" as arg 1 the message will
actually tell you how the client is NOT configured.
If the code was changed to use these two booleans then they could also be
passed as the args to the log message and I think this method would be
easier to understand.
The new code would be this:
> private void concurrencyConfigurationCheck(VersionTag tag) {
> if (this.concurrencyMessageIssued) {
> return;
> }
> boolean serverConcurrencyChecksEnabled = this.concurrencyChecksEnabled;
> boolean clientConcurrencyChecksEnabled = tag != null;
> if (clientConcurrencyChecksEnabled != serverConcurrencyChecksEnabled) {
> this.concurrencyMessageIssued = true;
> logger.info(LocalizedMessage.create(
>
> LocalizedStrings.LocalRegion_SERVER_HAS_CONCURRENCY_CHECKS_ENABLED_0_BUT_CLIENT_HAS_1_FOR_REGION_2,
> new Object[] {serverConcurrencyChecksEnabled,
> clientConcurrencyChecksEnabled, this}));
> }
> }
What do you think?
On Fri, Apr 14, 2017 at 6:50 PM, Kirk Lund <kl...@apache.org> wrote:
> I can't quite make out what the conditional is actually supposed to be
> checking in the second half but it definitely looks wrong to me. Anyone
> familiar with this method or what it's supposed to be doing?
>
> private void concurrencyConfigurationCheck(VersionTag tag) {
> // TODO: double negative in next line must be a bug
> if (!this.concurrencyMessageIssued && *((tag != null) !=
> this.concurrencyChecksEnabled)*) {
> this.concurrencyMessageIssued = true;
> logger.info(LocalizedMessage.create(
>
> LocalizedStrings.LocalRegion_SERVER_HAS_CONCURRENCY_CHECKS_
> ENABLED_0_BUT_CLIENT_HAS_1_FOR_REGION_2,
> new Object[] {!this.concurrencyChecksEnabled,
> this.concurrencyChecksEnabled, this}));
> }
> }
>
