inspection " condition is always true" not accurate

Hello Thibaut,

T> for the following code
T>
T> boolean ret = true;
T> for (Iterator<Map.Entry<LogEvent.Part, Predicate>> it =
T> predicatesByParts.entrySet().iterator(); it.hasNext() && ret;) {
T> Map.Entry<LogEvent.Part, Predicate> entry = it.next();
T> LogEvent.Part part = entry.getKey();
T> Predicate predicate= entry.getValue();
T> ret &= predicate.evaluate(event.getPart(part));
T> }
T> The inspection tells me that on the last line, ret is always true,
T> which is wront, as the right side may be false
T>
T> Should I open a JIRA issue ?

Please do.

--
Dmitry Jemerov
Software Developer
http://www.jetbrains.com/
"Develop with Pleasure!"

I'd like to take this opportunity to mention this false positive in
"Constant conditions & Exceptions" too:
http://www.jetbrains.net/jira/browse/IDEABKL-3332

It has been in the backlog for a while and it would like to come out:-)

Bas

Dmitry Jemerov wrote:

Hello Thibaut,

T> for the following code
T>
T> boolean ret = true;
T> for (Iterator<Map.Entry<LogEvent.Part, Predicate>> it =
T> predicatesByParts.entrySet().iterator(); it.hasNext() && ret;) {
T> Map.Entry<LogEvent.Part, Predicate> entry = it.next();
T> LogEvent.Part part = entry.getKey();
T> Predicate predicate= entry.getValue();
T> ret &= predicate.evaluate(event.getPart(part));
T> }
T> The inspection tells me that on the last line, ret is always true,
T> which is wront, as the right side may be false
T>
T> Should I open a JIRA issue ?

Please do.

Thibaut wrote:

for the following code

boolean ret = true;
for (Iterator<Map.Entry<LogEvent.Part, Predicate>> it = predicatesByParts.entrySet().iterator(); it.hasNext() && ret;) {
Map.Entry<LogEvent.Part, Predicate> entry = it.next();
LogEvent.Part part = entry.getKey();
Predicate predicate= entry.getValue();
ret &= predicate.evaluate(event.getPart(part));
}


The inspection tells me that on the last line, ret is always true, which is wront, as the right side may be false

I would guess what the inspection really means in this case
is the value of "ret" on the right side of the "implicit"
assignment

ret = ret & predicate.evaluate(event.getPart(part));

which is indeed always true.

If that's the case, the wording should be a bit more clear
here; maybe it should say "'&=' can ge simplified to '='"
or something like that.

Regards,
Jens

interesting point !

Although I believe the current coding (using &=) gives a better description of the intent of the code : indeed computing a logical AND operation on a set. An optimization has been written to stop checking when a false value shows up, but should it cause the &= to be replaced by a = (which would effectively be equivalent).

Anyway, you make a point, the inspection advice was not wrong