Rubberduck: Inspection for verbose boolean assignment within If Else condition block expression

Created on 8 Sep 2017  路  5Comments  路  Source: rubberduck-vba/Rubberduck

It's common to see boolean assignments like this on SO:

  Dim dayOfMonthIsEven As Boolean
  If Day(Now()) Mod 2 = 0 Then
    dayOfMonthIsEven = True
  Else
    dayOfMonthIsEven = False
  End If

But that's 5 lines of code to resolve the boolean condition and assign a boolean value, that can be achieved more efficiently, concisely and clearly, in a single line assignment:

  Dim dayOfMonthIsEven As Boolean
  dayOfMonthIsEven = Day(Now()) Mod 2 = 0

RD should be able to identify these long-winded assignments, and rewrite them more efficiently as a boolean expression.

We'll also need to identify the equivalent single line If statement:

  If Day(Now()) Mod 2 = 0 Then dayOfMonthIsEven = True Else dayOfMonthIsEven = False

And also need to identify the equivalent IIf statement:

  dayOfMonthIsEven = IIf(Day(Now()) Mod 2 = 0, True, False)

RD would also need to account for the reversed boolean logic that might require the expression to be flipped with a Not operator (or some other expression adjustment):

  Dim dayOfMonthIsEven As Boolean
  If Day(Now()) Mod 2 = 1 Then
    dayOfMonthIsEven = False
  Else
    dayOfMonthIsEven = True
  End If

Could become:

dayOfMonthIsEven = Not Day(Now()) Mod 2 = 1 

Or better still (although this would be much harder):

dayOfMonthIsEven = Day(Now()) Mod 2 = 0 
code-path-analysis enhancement feature-inspections

Most helpful comment

@Inarion I was mentioning in chat, that I only intend to introduce RedundantParensInspection after we've subclassed the code panes and managed to get inspection results show up as red/blue/green/dotted squiggles (depending on inspection severity) in the code pane itself - I'd use inspection results of RedundantParensInspection to tweak syntax highlighting and make the redundant parentheses appear dimmed / grayed-out in the editor. Placing the caret on one such redundant parenthesis would pop a "Fix" button (in-place!) from which users could pick a "Remove redundant parentheses" quick-fix.

All 5 comments

I guess we could also spot the instances where only the True assignment is made, on the assumption that the variable default value is False:

  Dim dayOfMonthIsEven As Boolean 'Defaults to False
  If Day(Now()) Mod 2 = 0 Then
    dayOfMonthIsEven = True
  End If

That still benefits from being rewritten, in terms of performance/readability, but it also removes a potential variable is not assigned in all code paths inspection.

FWIW and this is just a minor stylistic point, I'd prefer to wrap the rewritten boolean expression in a () such as dayOfMonthIsEven = (Day(Now()) Mod 2 = 0). Otherwise, it looks "funny" having a line with apparently 2 assignment operators when in actuality, only one of them is an assignment and other is the equality check.

@bclothier note that when a RedundantParensInspection is implemented, these will pop a result.

However, this begs the question: Can something be considered redundant if it helps clarity? (Although I have to admit, the concept of "clarity" might be partially - or even entirely - subjective.)

@Inarion I was mentioning in chat, that I only intend to introduce RedundantParensInspection after we've subclassed the code panes and managed to get inspection results show up as red/blue/green/dotted squiggles (depending on inspection severity) in the code pane itself - I'd use inspection results of RedundantParensInspection to tweak syntax highlighting and make the redundant parentheses appear dimmed / grayed-out in the editor. Placing the caret on one such redundant parenthesis would pop a "Fix" button (in-place!) from which users could pick a "Remove redundant parentheses" quick-fix.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

philippetev picture philippetev  路  3Comments

ghost picture ghost  路  3Comments

retailcoder picture retailcoder  路  4Comments

Gener4tor picture Gener4tor  路  3Comments

Gener4tor picture Gener4tor  路  3Comments