rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.91k stars 300 forks source link

Flag redundant dereferencing (library-specific) #4895

Open retailcoder opened 5 years ago

retailcoder commented 5 years ago

What Rubberduck should be able to locate superfluous dereferencing of certain objects in the Excel object model. However categorizing this under "language opportunities" would be conflating VBA (the language) with the Excel type library / object model, so a new "library opportunities" category might be warranted for it.

Why The Excel object model is extremely flexible; default members returning a workbook or worksheet's name, can easily be used to get the very same object from a collection object. Such calls are redundant and should be avoided.

Examples

Redundant worksheet dereferencing:

Public Sub DoSomething(ByVal ws As Worksheet)
    Worksheets(ws).Range("A1").Value = 42 ' ws is already a Worksheet
End Sub

Redundant workbook dereferencing:

Public Sub DoSomething(ByVal wb As Workbook)
    Workbooks(wb).Worksheets(1).Range("A1").Value = 42 ' wb is already a Workbook
End Sub

Redundant range dereferencing:

Public Sub DoSomething(ByVal target As Range)
    Range(target.Address).Value = 42 ' target is already a Range
End Sub

Note that this last example assumes the objective is to get a cell on the same sheet as the supplied Range object - but implicitly refers to the active sheet. Inspection info should probably mention that.


QuickFixes A quickfix should be available to remove the redundant code, depending on the type of object being dereferenced:

  1. Use existing object reference

    Example code, after quickfix is applied:

    Public Sub DoSomething(ByVal ws As Worksheet)
        ws.Range("A1").Value = 42
    End Sub
    Public Sub DoSomething(ByVal wb As Workbook)
        wb.Worksheets(1).Range("A1").Value = 42
    End Sub
    Public Sub DoSomething(ByVal target As Range)
        target.Value = 42
    End Sub

Resources

retailcoder commented 5 years ago

Maybe this should be 3 (related) inspections instead of one?