After inspecting the history of that line, my main conclusion is that there has been some incompetent programming at work.
-
That line is gratuitously convoluted. The general form
a? true : bfor
boolean a, bis equivalent to the simplea || b -
The surrounding negation and excessive parentheses convolute things further. Keeping in mind De Morgan’s laws it is a trivial observation that this piece of code amounts to
if (!_ok && Math.random() <= 0.1) return res; -
The commit that originally introduced this logic had
if (_ok == true) { _logger.log( Level.WARNING , "Server seen down: " + _addr, e ); } else if (Math.random() < 0.1) { _logger.log( Level.WARNING , "Server seen down: " + _addr ); }—another example of incompetent coding, but notice the reversed logic: here the event is logged if either
_okor in 10% of other cases, whereas the code in 2. returns 10% of the times and logs 90% of the times. So the later commit ruined not only clarity, but correctness itself.I think in the code you have posted we can actually see how the author intended to transform the original
if-thensomehow literally into its negation required for the earlyreturncondition. But then he messed up and inserted an effective “double negative” by reversing the inequality sign. -
Coding style issues aside, stochastic logging is quite a dubious practice all by itself, especially since the log entry does not document its own peculiar behavior. The intention is, obviously, reducing restatements of the same fact: that the server is currently down. The appropriate solution is to log only changes of the server state, and not each its observation, let alone a random selection of 10% such observations. Yes, that takes just a little bit more effort, so let’s see some.
I can only hope that all this evidence of incompetence, accumulated from inspecting just three lines of code, does not speak fairly of the project as a whole, and that this piece of work will be cleaned up ASAP.