Code inspection - Named Range Reference

Question

After running the code inspections in Rubberduck 2.0.11.2453, there are 4 Range references that get tagged as:

Member 'Range' implicitly references ActiveSheet

The ranges in question are referring to Named Ranges. Is it necessary to qualify a named range reference?

Private Sub RunORatio(ByVal TabNum As Integer)
  Dim Start As Integer, Cat As Integer, iMth As Integer, CurrentRow As Integer, Report As Integer
  Dim wsORatio As Worksheet, wsData As Worksheet, wsMacro As Worksheet
  Dim sMap As String, Test As String

  With ActiveWorkbook
    Set wsMacro = .Worksheets("Macro")
    Set wsORatio = .Worksheets("ORatio" & TabNum)
    With wsORatio
      sMap = "oratio" & TabNum & "map"           
      For CurrentRow = 1 To Range(sMap).Rows.Count             '<---1 here
        Test = Range(sMap).Cells(CurrentRow, 1)                '<---1 Here
        Set wsData = ActiveWorkbook.Worksheets(Test)
        Start = Range(Range(sMap).Cells(CurrentRow, 2)).Row    '<---2 Here
        Report = wsMacro.Range(sMap).Cells(CurrentRow, 3)
        For Cat = 0 To 12
          For iMth = 1 To 12
            wsORatio.Cells(Report + Cat, 7 + iMth) = wsData.Cells(Start + Cat, 37 + iMth)
          Next iMth
        Next Cat
      Next CurrentRow
    End With
  End With
End Sub

Answer 1

Disclaimer: I'm heavily involved in the development of Rubberduck.

Consider this common mistake:

lastRow = Worksheets("Sheet12").Cells(1, Rows.Count).End(xlUp).Row

Rows is unqualified, and therefore implicitly refers to the active sheet and therefore Rows.Count isn't necessarily the row count on "Sheet12". The code might work, but it could also result in a subtle bug where lastRow doesn't have the correct value because of this, depending on the content of the active sheet.

Or this one:

ActiveWorkbook.Worksheets("SummarySheet") _
    .ListObjects("Table1").Sort.SortFields.Add _
        Key:=Range("Table1[[#All],["Date]]"), _    
        SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortTextAsNumbers

See it? Because the Key parameter isn't qualified, the call is going to fail at run-time with error 1004 - "Method 'Range' of object '_Global' failed". That's 169 Stack Overflow questions. "Error 1004" yields 1465 Stack Overflow questions.

Implicit references to the active worksheet are a very common cause of bugs.

---

Rubberduck's VBA code inspections are, like ReSharper's C# static code analysis, hints/suggestions. The tool is telling you that it's possible there's something here that could cause problems, or that makes the code less explicit than it could be.

Do you need to fully qualify every single Range call? Of course not - Rubberduck is merely letting you know that in these instances, ActiveSheet is implicitly being referenced, that's all there is to it.

You can always tell Rubberduck "look, I know what I'm doing", using the "Ignore once" quick-fix:

That "fix" inserts a special comment (internally, Rubberduck calls them "annotations") that instructs the inspection to ignore specific results, while leaving the inspection enabled:

  With ActiveWorkbook
    Set wsMacro = .Worksheets("Macro")
    Set wsORatio = .Worksheets("ORatio" & TabNum)
    With wsORatio
      sMap = "oratio" & TabNum & "map"
      '@Ignore ImplicitActiveSheetReference           
      For CurrentRow = 1 To Range(sMap).Rows.Count             
        '@Ignore ImplicitActiveSheetReference           
        Test = Range(sMap).Cells(CurrentRow, 1)                
        Set wsData = ActiveWorkbook.Worksheets(Test)
        '@Ignore ImplicitActiveSheetReference           
        Start = Range(Range(sMap).Cells(CurrentRow, 2)).Row    
        Report = wsMacro.Range(sMap).Cells(CurrentRow, 3)
        For Cat = 0 To 12
          For iMth = 1 To 12
            wsORatio.Cells(Report + Cat, 7 + iMth) = wsData.Cells(Start + Cat, 37 + iMth)
          Next iMth
        Next Cat
      Next CurrentRow
    End With
  End With

These annotations have the advantage of telling the reader (future you, or whoever takes your code over) that there's something going on here.

Future versions will eventually support specifying @Ignore annotations once at module-level, to ignore all results of a particular inspection in an entire module.

---

Note that the inspection is under the Maintainability and Readability Issues category. Range("DefinedName") isn't half as explicit and fail-safe as:

ActiveWorkbook.Names("DefinedName").RefersToRange

Which gives you the same range, and reads like it's actually pulling a named range scoped at workbook level.

Answer 2

Disclaimer: I'm also involved in the development of Rubberduck.

As @Mat'sMug already pointed out, the inspection is providing you information about the code. What you do with that information is a matter of preference, coding sytle, etc.

In this case, the inspection isn't telling you that there is an error in your code, it's telling you that there is a potential that implicit object references will cause your code to behave in unexpected ways. The takeaway from this specific inspection is that you are letting Excel decide how to interpret which object you are referring to.

From the documentation for Application.Range:

When used without an object qualifier, this property is a shortcut for ActiveSheet.Range (it returns a range from the active sheet; if the active sheet isn't a worksheet, the property fails).

The last sentence is the first reason you shouldn't ignore this inspection - Range without a qualifier will throw if you have a Chart selected. This is the same reason why you should use Workbook.Worksheets(foo) instead of Workbook.Sheets(foo) if you are assigning to a Worksheet (this is actually on the Rubberduck inspection wish-list).

The second reason is related to the first. As you correctly point out in the comments, "the named range has a sheet name as part of its reference", or rephrased "a named range can be assumed to be unique". But it doesn't have to be unique if you have more than one workbook open. Since ActiveSheet is always returned from the ActiveWorkbook at the time of the call, if the Workbook the code expects is not active, this code will either throw or return the incorrect Range (if the other Workbook coincidentally contains a Range with the same name).

Keep in mind that although the With block captures a hard reference to ActiveWorkbook, Application does not - and ActiveWorkbook can change during the course of your code's execution. This all ties back to the reasons why you should avoid using the ActiveFoo objects - they introduce the possibility that errors will result from otherwise "correct" code.

The solution is simple. Just add the dot in front of them. That way they'll be calling Workbook.Range via the captured hard reference from the With block instead of Application.Range.