2010/5/26 sebb <seb...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
