2010/5/26 sebb <sebbaz@gmail.com>:
>> The "removed" variable is only checked whether it is zero or some
>> positive value. So, the actual value of the variable before the
>> increment operation is irrelevant. It could be written as "removed =
>> 1;" and assignment is atomic.
>
> In that case, why not do so - or make it a boolean?
>
If you see the old code of "isRemoved()" -- before this patch -- it
used to check "if (removed > 1)". So, "0", "1", and ">1" were distinct
states.
I agree with you that this private field can now be made boolean.
>> Are there any observable consequences? If they are, you may file an issue.
>
> A brief scan of the code suggests not, unless it is possible for the
> counter to overflow.
>
> But it is confusing to have a method called incrementRemoved() and a
> corresponding counter which is actually being used purely as a flag.
>
0 -> 1 is an increment, as well as false -> true. ;)
How the counter is implemented should not bother one who calls this
method. I am fine with its name. (I have thought of a pair of
alternatives, but all they are more confusing than the current name).
> May I suggest adding a comment to document this for future maintainers?
>
;)
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
|