Rubberduck: `Implicit reference to Active Sheet` inspection needs an additional QuickFix option

Created on 22 Feb 2017  路  7Comments  路  Source: rubberduck-vba/Rubberduck

There are times, particularly when writing convenience macros that reside in PERSONAL.XLSB that I want to reference the ActiveSheet.

As an added QuickFix option, it would be nice to have a Make the reference explicit in addition to the current Ignore once. I think that making it explicit is probably a better bet in the long run than ignoring the inspection - it should help speed up processing (wouldn't it?), and helps someone who doesn't have RD* to better understand that you're intending to reference the ActiveSheet.

Additionally, the "description pane" of the CI window offers the option to Disable this inspection. An added bonus would be Ignore in this module. This might be more difficult to implement, but would, essentially, be Ignore once for every occurrence in the current code module.

*That person should be introduced to RD, and the user should be flogged for failing to do so, but that's a different inspection result.

code-path-analysis enhancement feature-inspections

Most helpful comment

I am trying to think through this like an accountant who is becoming something of a VBA programmer who's trying to learn better coding habits rather than an experienced programmer being tortured by having to write VBA...

I absolutely love that perspective. Thank you for bringing it to the discussion.

All 7 comments

I'm not exactly sure what that would look like. Are you thinking something along the lines of introducing a local variable? I.e:

'Before
Sub Foo()
    Debug.Print Range("A1").Value
End Sub

Apply quick-fix:

'After
Sub Foo()
    Dim localSheet As Worksheet
    Set localSheet = ActiveSheet
    Debug.Print localSheet.Range("A1").Value
End Sub

@comintern I'd just make the reference explicit.

Before:

vb Sub Foo() Debug.Print Range("A1").Value End Sub

After:

vb Sub Foo() Debug.Print ActiveSheet.Range("A1").Value End Sub

Whether ActiveSheet being dereferenced half a dozen times in the same scope is a problem, should be a separate inspection IMO.

I was thinking along the lines of @retailcoder suggestion. Though I can see the potential performance hit of dereferencing ActiveSheet, so possibly options for both - make it easier for the user to decide the most appropriate solution for his situation.

Also, it does serve as a teaching tool by giving options and their advantages/drawbacks. Perhaps a link to a rd.wiki reference that has the explanation instead of in the inspection result.

I am trying to think through this like an accountant who is becoming something of a VBA programmer who's trying to learn better coding habits rather than an experienced programmer being tortured by having to write VBA...

I am trying to think through this like an accountant who is becoming something of a VBA programmer who's trying to learn better coding habits rather than an experienced programmer being tortured by having to write VBA...

I absolutely love that perspective. Thank you for bringing it to the discussion.

@daFreeMan the hit is the same if the dereferencing is explicit or implicit really.

The inspection isn't solely about the dereferencing issue anyways. Making the dependency explicit accomplishes at least one part of the justification for the issue.

The repeated dereferencing may also be intentional. If it isn't we can still flag "repeated use of the same object reference" as soon as we get proper call graphs (which is a different kind of crazy to deal with)

Was this page helpful?
0 / 5 - 0 ratings