useSimplifiedLogicExpression is actually making logic expressions more complex

[ricardobeat]

The useSimplifiedLogicExpression rule seems a bit arbitrary and doesn't result in clearer code. For example, it suggests replacing this:

if (!width || !height) {
  target.width = "100%";
} else {
  target.width = width;
  target.height = height;
}

with this (double parenthesis added by the formatter):

if ((width || height)) {
  target.width = width;
  target.height = height;
} else {
  target.width = "100%";
}

Not only this is not an improvement in readability - the original logic "if either parameter is missing, use this default" is simpler - but it adds visual noise with the extra parenthesis for no apparent reason.

In a similar case of "if both parameters are missing, use this default" logic, an equally unexpected suggestion is given:

if (!width && !height) {
  styleWidth = "100%";
} else {
  ...
}

becomes

if (!(width || height)) {
  styleWidth = "100%";
} else {
  ...
}

This makes the condition a lot harder to understand, increasing the complexity of the code.

Another example was provided in #3770.

[ematipico]

There's one thing that maybe it hasn't been highlighted in the description of the rule.

This rule applies the De Morgan's theorem, which has been proofed to reduce the complexity of logical expressions:

• https://runestone.academy/ns/books/published/csjava/Unit3-If-Statements/topic-3-6-DeMorgan.html
• https://en.m.wikipedia.org/wiki/De_Morgan%27s_laws

Given for granted the validity of the theorem, I suppose the discussion should be focused on:

• how readable is the code suggested;
• why the suggesion feels more complex;
• when the suggestion feels more complex;

I agree that sometimes the suggestion might feel more complex, and that's probably because my brain works in a different way compared to the theorem. On the other hand the theorem is scientifically proofed.

[leops]

This is one of the rules I intended to remove from the recommended set in v11, but it seems like I missed it in the refactor

[arendjr]

I do agree with the rule’s suggestion in the first example. This seems significantly more readable to me:

if ((width || height)) {
  target.width = width;
  target.height = height;
} else {
  target.width = "100%";
}

Than the original:

if (!width || !height) {
  target.width = "100%";
} else {
  target.width = width;
  target.height = height;
}

I also tend to avoid negatives in my code to avoid cognitive load, which may help explain why I prefer the former.

I do agree it’s odd the unnecessary parentheses get added though, but I assume that’s a minor issue that can be separately solved.

As for the second example:

if (!width && !height) {
  styleWidth = "100%";
} else {
  ...
}

I agree this should not become:

if (!(width || height)) {
  styleWidth = "100%";
} else {
  ...
}

In my opinion it should again try to avoid the negation so that it would end up with the same suggestion as the first example.