8

An Excel file includes VBA-coded user-defined functions (UDFs) that are deployed in tables (VBA listobjects). Now, for reasons that escape me, if the UDF module contains Range variables that are declared outside the scope of any sub or function, I get a very dramatic warning when the file is opened: "Automatic error -- Catastrophic failure".

"Catastrophic" seems like an exaggeration because after the warning is dismissed, the file seems to work correctly. But I would still like to understand what the issue is. I have managed to replicate the issue with an MVC example as follows. I'm running Excel 2016 (updated) on Windows 10.

There are two tables (i.e. VBA listobjects): Table 1 lists "items" and Table 2 lists "item features" (both tables were generated by selecting the data and clicking Table on the Insert tab). Table 2 has a UDF called ITEM_NAME() in the field Item_Name that returns the item name as a function of the item ID, see the screenshot:

enter image description here

The function ITEM_NAME() is essentially a wrapper around the regular worksheet functions INDEX and MATCH, as in the following code:

Option Explicit

Dim mrngItemNumber As Range
Dim mrngItemName As Range

Public Function ITEM_NAME(varItemNumber As Variant) As String
' Returns Item Name as a function of Item Number.
    Set mrngItemNumber = Sheets(1).Range("A4:A6")
    Set mrngItemName = Sheets(1).Range("B4:B6")
    ITEM_NAME = Application.WorksheetFunction.Index(mrngItemName, _
    Application.WorksheetFunction.Match(varItemNumber, mrngItemNumber))
End Function

So, to repeat, with this setup I get the Automation error when the file is opened. But the error disappears when I do any of the following:

  1. Move the declarations into the scope of the function. This solution is not attractive since it requires many more lines of code, one for each UDF, and there are many.

  2. Change the variable type from Range to something else, for example Integer (so the function will obviously not work).

  3. Convert Table 2 to an ordinary range (i.e. remove the table). This is also an inconvenient solution since I really want to use the Table features for other purposes in my code.

  4. Remove the function ITEM_NAME() from Table 2. (Obviously no attractive option..)

What's going on? Why do I get the error message? And why does the file still seem to work properly despite the warning? Is there a workaround that I've missed?

I suspect it might have something to do with how sheet objects and listobjects interact, but not sure. A possible hint is provided in this answer to another question:

If you want to reference a table without using the sheet, you can use a hack Application.Range(ListObjectName).ListObject.

NOTE: This hack relies on the fact that Excel always creates a named range for the table's DataBodyRange with the same name as the table.

Similar problems have been reported elsewhere (at Stackoverflow and Microsoft Technet), but not with this particular flavor. Suggested solutions include checking for broken references or other processes running in the background, and I've done that to no avail. I can also add that it makes no difference whether the function ITEM_NAME is entered after Table 2 is created rather than before; the only difference is that it uses structured references in that case (as in the screenshot above).

UPDATE: Inspired by @SJR's comments below I tried the following variation of the code, where a ListObject variable is declared to store the table "Items". Note that the Range declarations are now inside the scope of the function, and that only the ListObject declaration is outside. This also generates the same Automation error!

Option Explicit

Dim mloItems As ListObject

Public Function ITEM_NAME(varItemNumber As Variant) As String
' Returns Item Name as a function of Item Number.
    Dim rngItemNumber As Range
    Dim rngItemName As Range
    Set mloItems = Sheet1.ListObjects("Items")
    Set rngItemNumber = mloItems.ListColumns(1).DataBodyRange
    Set rngItemName = mloItems.ListColumns(2).DataBodyRange
    ITEM_NAME = Application.WorksheetFunction.Index(rngItemName, _
    Application.WorksheetFunction.Match(varItemNumber, rngItemNumber))
End Function

UPDATE 2: The problem now seems to be solved, but I'm not much wiser as to what actually caused it. Since no one could replicate (not even friends of mine who opened the same file on different systems), I began to think that it was a local issue. I tried repairing Excel and then even reinstalled the complete Office package from scratch. But the issue still persisted, both with my MCV files used to create the example above and the original file where I discovered the problem.

I decided to try to create a new version of the MCV example where, inspired by AndrewD's answer below, I used .ListObjects() to set the range instead of using .Range(). This did indeed work. I will probably adapt that solution for my work (but see my comments under AndrewD's question explaining why I might prefer .Range().)

In order to double check that this solution worked, I set about to create two new files, one to replicate my own example as described above, and one where the only difference would be the switch to ListObjects(). In the process, I noted that I had actually indented the Range declarations at the beginning of the code in my original file, like so:

Option Explicit

    Dim mrngItemNumber As Range
    Dim mrngItemName As Range

Public Function ITEM_NAME(...

Without thinking much about this, I created the new file but without indentation. So that would be an exact copy of the previous file (and the given example above), but without indentation. But behold, with this file I could not replicate the Automation error! After inspecting both files I noted that the only difference was indeed indentation, so I put the indentation back again in the new file expecting it to generate the Automation error again. But the problem did not reappear. So then I then removed the indentation from the first file (used to create the example above), and now the Automation error disappeared from that file as well. Armed with this observation, I went back to my real file where I first discovered the issue and simply removed the indentation there too. And it worked.

So to summarize, after removing the indentation of the Range declarations I fail to recreate the Automation error in any of the three files that had generated it before. And moreover, the problem does not reappear even if I put the indentation back in place again. But I still don't understand why.

Thanks everyone who took time to look at this and shared valuable ideas.

Egalth
  • 962
  • 9
  • 22
  • Can't replicate your error so probably can't help much! Do you have any other code in the file? Esp in the workbook open event? – SJR Sep 05 '17 at 15:26
  • Also, don't know it will make any difference but why not refer to the table columns in your code rather than hard-coded ranges? – SJR Sep 05 '17 at 15:32
  • No, the file I used for the MVC example contains nothing else except what's shown above. – Egalth Sep 05 '17 at 15:38
  • Yes, good point about using column headers. In my "live" file there are no hard-coded ranges, but named ranges, but I've excluded that here to make the example easier to replicate. However, I like to use named ranges so that I can change the name of a header without changing the code. – Egalth Sep 05 '17 at 15:41
  • You can refer to a column like so `Set mrngItemNumber = Sheets(1).ListObjects("Items").ListColumns(1).DataBodyRange` so unaffected if you change its name. When you get the automation error is anything highglighted in the VBE? – SJR Sep 05 '17 at 15:48
  • Thanks, that's more elegant of course. I might adopt that approach eventually, but I'm still in the design process and moving columns around a lot so the index number might also change.. Highligt? Yes, the first variable is highlighted, `mrngItemNumber`. – Egalth Sep 05 '17 at 15:52
  • I tried implementing your suggestion with column headers. In the process I also added a ListObject variable before the range variables, like so: `Dim mloItems As ListObject`. The assign statements now look like this: `Set mrngItemNumber = mloItems.ListColumns(1).DataBodyRange`. This did not affect the issue, but now the highlight has moved to `mloItems` after the automation error (so it¨s still the first declared variable that's highlighted). – Egalth Sep 05 '17 at 16:08
  • Clutching at straws, but what if you use `Public mrngItemNumber As Range`? – SJR Sep 05 '17 at 16:25
  • Yes, I tried variations of Dim/Public/Private, but all generate the same error. Please also note my update, which adds to the intrigue. – Egalth Sep 05 '17 at 16:28
  • My first suggestion is to run Excel in safe mode (by holding down `Ctrl` when starting) to see if this temporarily fixes the problem. Secondly, try on a different computer, and possibly a different version of Excel. It's also useful to tag these type of questions with the version of Excel you are using, PS Can't replicate the issue here with Excel 2007, either. – robinCTS Sep 05 '17 at 17:46
  • Have you tried to directly evaluate the name of the table: `ITEM_NAME = WorksheetFunction.Lookup(varItemNumber, [Table1[Item_ID]], [Table1[Item_name]])` ? – Florent B. Sep 05 '17 at 18:01
  • Then try changing `Variant` to `Range` in your function declaration. (Like so: `Public Function ITEM_NAME(varItemNumber As Range) As String`.) – robinCTS Sep 05 '17 at 18:07
  • @robinCTS thanks, both very reasonable suggestions, I tried safe mode and switching Variant to Range but the problem persists. I'm running Excel 2016 on Windows 10. I'm waiting for someone to test on another machine. – Egalth Sep 05 '17 at 18:35
  • Next, before opening, try switching the calculation mode to manual, then opening file. Add `Private Sub Workbook_Open()` `Application.Calculation = xlManual` `Application.Calculation = xlCalculationAutomatic` `End Sub` to the `WorkBook.Open` event handler in `ThisWorkbook. Save. Quit. Re-open. – robinCTS Sep 05 '17 at 18:48
  • @FlorentB, using Lookup instead of Index-Match works indeed in this example, but doesn't address the problem at hand. – Egalth Sep 05 '17 at 19:10
  • @Egalth, it seems like it answers you question: "Is there a workaround that I've missed?". Note that you can use the name of the table the same way with `Index` and `Match`. – Florent B. Sep 05 '17 at 19:19
  • Ok @robinCTS, done that; issue persists (I tried it exactly as you stated, and also leaving out the second line with `xlCalculationAutomatic`) – Egalth Sep 05 '17 at 19:22
  • @FlorentB. Ok, got it, but even with Lookup I do get the "Catastrophic" Automation error when the file is opened, so it's not the workaround I'm looking for. – Egalth Sep 05 '17 at 19:24
  • 1
    I think I've found a solution, now that the basics have been done. Will post it as an answer when it's finished. – robinCTS Sep 05 '17 at 19:29
  • @robinCTS were you able to replicate? – Egalth Sep 05 '17 at 19:30
  • No. That's why I got you to try all the simple possible solutions first. – robinCTS Sep 05 '17 at 19:39
  • @FlorentB. Just a small note regarding LOOKUP vs. INDEX-MATCH, from Microsoft Support: "You can use a combination of the INDEX and MATCH functions, a combination of the OFFSET and MATCH functions, HLOOKUP, or VLOOKUP to provide the same functionality as LOOKUP. None of these choices require that the lookup table be sorted, unlike the LOOKUP function. " https://support.microsoft.com/en-us/help/181212/how-to-use-the-lookup-function-with-unsorted-data-in-excel – Egalth Sep 05 '17 at 20:50
  • 1
    @Egalth, the identifiers from your example are sorted. That's why i suggested `LOOKUP` since it's less expensive than `INDEX+MATCH` ( O(log n) vs O(n^2) ). – Florent B. Sep 05 '17 at 20:58
  • 1
    @Egalth I couldn't reproduce your error in Excel 2016 :( I left the table names as Table1 and Table2 and created the UDF in a module called Module1. But I do question your approach, because UDF's are performance killers. I'd recommend using just a formula i.e. "=INDEX(Table1[Item_name],MATCH([@[Item_ID]],Table1[Item_ID]))" (or "=VLOOKUP([@[Item_ID]],Table1,2,FALSE)", though I prefer the former in case columns are re-arranged). Not only does Excel use this WAY more efficiently, but if you rename tables or columns or rearrange columns it will continue to work. And it stays an XLSX. HTH – AndrewD Sep 07 '17 at 02:15
  • As to an explanation, my guess is Excel is attempting to calculate the function & getting in a twist when it works through the calcs in a certain manner. Excel's algorithms will attempt to recalculate everything as you open, thereafter only calculating if something changes or is "volatile". It may be the initial calc is done in a different manner to subsequent calcs. You've not mentioned whether the tables are normally sourced from a query/external source, but this could also affect things (hence the calc option "Auto Except Tables" under the Formulas ribbon). – AndrewD Sep 07 '17 at 02:23
  • One last idea to work out what is going on. If you still have a faulty copy of the example file you posted, export the module with a .txt extension and open it in Notepad/favourite **text** editor. Compare it with the exported module from the new working copy. Post any differences here. I presume you are unable to generate a new faulty example file? Exactly how did you create the first faulty one? (Did you use a fresh workbook? Did you export/import the code module from your real file or did you copy/paste the code text or did you type it in?) – robinCTS Sep 09 '17 at 05:19
  • (1/2) The first faulty example file is still faulty. I created a new workbook from scratch and copy-pasted the relevant code from my real file. The error occurred as expected. Then I proceeded to recreate a second file from scratch in yet another new workbook where I typed everything from scratch and also used different data. That's what I used for the example posted here, and that's the file that was "fixed" by tweaking the indentation. – Egalth Sep 09 '17 at 20:29
  • (2/2) Now, to follow your lead and compare differences in a text editor, I saved a new copy of the first file and intended to tweak the indentation and then export that module as txt as well. But when I saved that new copy, the Automation error disappeared, althoug the code and the data were identical to the original file. (And moreover, the very first faulty file actually didn¨t have indented declarations. It was only the second file -- so tweaking the indentations was one way to solve the issue, but clearly not the only way.) – Egalth Sep 09 '17 at 20:41
  • And yes, you'r right, I am indeed unable to generate a new faulty file. I just tried typing out a new file from scratch that was identical to the one I used for the exampel above (with the indentation), but I did not get the Automation error. – Egalth Sep 09 '17 at 20:43
  • I'm still curious about the differences in the exported modules. It might be better to compare them in a hex editor or use a file diff program. I was originally looking for a difference in hidden (text) attributes, but maybe there exist non-printable characters. Did you export and compare the two modules and were there any differences? I would appreciate If you could upload the still-faulty, the was-faulty-but-now-fixed, and the brand-new-always-worked workbooks as I would like to investigate them further. – robinCTS Sep 10 '17 at 01:26
  • 1
    I've run into similar issues many times in Excel VBA - a module suddenly goes "weird" e.g. once it refused to step onto a line of code, like it was stepping through an earlier version of the file. Do what I may, the damned thing wouldn't compile again. Export & re-import into a new workbook, still an issue. New workbook, copy-paste the text and...viola! I suspect MS have built in some "clever" compiling like they use in .Net where it only compiles changes...and now and again it loses the plot :( – AndrewD Jul 14 '20 at 12:38

3 Answers3

1

OK. This workaround should work.

If When it does, there are a few issues and caveats to address.

I'll also post explanations.

Install the code in the ThisWorkbook module.

Code:

Private Sub Workbook_BeforeClose(Cancel As Boolean)

  Dim rngCell As Range

  For Each rngCell In ActiveSheet.UsedRange.SpecialCells(xlCellTypeFormulas)
    With rngCell
      If .FormulaR1C1 Like "*ITEM_NAME*" _
      And Left$(.FormulaR1C1, 4) <> "=T(""" _
      Then
        .Value = "=T(""" & .FormulaR1C1 & """)"
      End If
    End With
  Next rngCell

End Sub

Private Sub Workbook_Open()

  Dim rngCell As Range

  For Each rngCell In ActiveSheet.UsedRange.SpecialCells(xlCellTypeFormulas)
    With rngCell
      If .FormulaR1C1 Like "*ITEM_NAME*" _
      And Left$(.FormulaR1C1, 4) = "=T(""" _
      Then
        .FormulaR1C1 = .Value
      End If
    End With
  Next rngCell

End Sub
robinCTS
  • 5,746
  • 14
  • 30
  • 37
  • Now I've tried, and it does indeed prevent the annoying Automation error. Clever workaround! I get the logic of converting formulas to text before close and restoring the formulas when the file is opened. But I'm reluctant to accept the answer since I still don't understand why this occurs. As written, it obviously only covers the one function `ITEM_NAME` and I need something more general. So what about those caveats you mentioned? – Egalth Sep 06 '17 at 14:52
  • Isn't the IF block redundant by the way? – Egalth Sep 06 '17 at 14:59
  • 1
    @Egalth The code, being a very simple test example, doesn't disable alerts, and since the file is "dirtied" in the close event handler, the confirmation save dialog is always displayed. If the user clicks cancel, the workbook is left in a contrary state. Closing again would result in a double `T()` function. (This is one of the to-be-mentioned issues). Thus the reason for the `T()` check in the `IF` block. Plus it's a good safety mechanism to leave in place once all known issues are addressed, since, being a Micro$oft product, who knows what will cause it to enter such a state again. – robinCTS Sep 09 '17 at 11:01
  • By the way: "...a good safety mechanism to leave in place" -- you mean to convert all formulas before close in the whole workbook like this as a general precaution? Or only for functions that are known to have caused issues? – Egalth Sep 09 '17 at 20:48
  • 1
    @Egalth My original intent was to only add those problem functions as they were found, one by one. However your idea is a good one. If you can stand the extra time required to open your workbooks (and all the other as yet unmentioned* issues are resolved or live-able with) then converting *all* the formulae is a much simpler and more robust solution! * I promise I *will* get around to updating this answer with the issues/caveats. Was just waiting for the last pieces of the puzzle as far as your original issue is concerned. I expect the answer might get rather long. – robinCTS Sep 10 '17 at 01:28
1

On a purely code level, why declare modular-level variables to store the ranges when you set them every single time? If you were caching the references and only setting them if Nothing I could understand...but then you would use a Static to reduce the scope.

My preference would be to not bother with the modular (or local/static) variables, replace the Worksheet.Name reference with Worksheet.CodeName (less likely to be changed and, if you compile after a rename you get an error) and refer to the table ranges via the ListObject and ListColumns (in case the table size changes).

' Returns the item name for the requested item ID.
Public Function ITEM_NAME(ByVal ItemID As Variant) As String
    ITEM_NAME = Application.WorksheetFunction.Index( _
                      Sheet1.ListObjects("Table1").ListColumns("Item_name").DataBodyRange _
                    , Application.WorksheetFunction.Match( _
                          ItemID _
                        , Sheet1.ListObjects("Table1").ListColumns("Item_ID").DataBodyRange _
                        ) _
                    )
End Function

But the most robust solution would be to avoid a UDF and use =INDEX(Table1[Item_name],MATCH([@[Item_ID]],Table1[Item_ID]‌​)) (VLOOKUP may be slightly faster but INDEX+MATCH is more robust).

AndrewD
  • 1,083
  • 9
  • 15
  • thanks for this, as you can see in my update above, the switch to ListObjects did indeed solve the issue, although I still don't understand why the problem arose in the first place. – Egalth Sep 08 '17 at 22:51
  • As for the modular-level declarations, that was a choice I made because since I will set them in several different UDFs (not shown in my question), I wanted to save myself the effort of having to declare them inside the scope of each UDF. Not sure if this is bad coding practice? – Egalth Sep 08 '17 at 22:52
  • In my live file I do indeed use CodeName instead of Name, I just didn't include it here to keep it as simple as possible. – Egalth Sep 08 '17 at 22:53
  • Regarding INDEX-MATCH vs. UDF: In my original file almost all the heavy lifting was done by INDEX-MATCH. The drawback was that since it is a very large file (over 50 sheets), the usage of INDEX-MATCH requires an intimate knowledge of the data structure of the whole file. And the idea is that others will work with the file. I will have several UDFs that will simplify the work for end users. They will be able to use the function anywhere without remembering anything more than its intuitive name. In addition, it will be called by other functions, so it's not meant only as a worksheet function. – Egalth Sep 08 '17 at 23:01
  • And finally: in my example the ranges were hard-coded. In my real file, the table columns are assigned to named ranges. These are dynamic and follow the table as it expands. This might seem redundant since they just mimick the column headers of the table, but there is one advantage: with named ranges I can and change the table columns without breaking the code. This is useful during development because there's a high chance that the logic (and hence the headers) of the tables will change in the process. Once the logic/design is ready, I agree that `ListObects` is to prefer. – Egalth Sep 08 '17 at 23:07
  • @Egalth If the only reason you are using module-level variables is to save the two declaration lines per UDF, then that *is* bad coding practice. – robinCTS Sep 09 '17 at 11:07
1

Declaring module-level variables simply to save the two lines in each UDF that would otherwise be required is indeed bad coding practice. However, if that is your thinking, why not go all the way and save four lines per UDF by avoiding setting them in each as well!

You can do this by using pseudo-constant functions as seen in the following code:

Option Explicit

Private Function rng_ItemNumber() As Range
    Set rng_ItemNumber = Sheet1.Range("A4:A6")
End Function
Private Function rng_ItemName() As Range
    Set rng_ItemName = Sheet1.Range("B4:B6")
End Function

Public Function ITEM_NAME(varItemNumber As Variant) As String
' Returns Item Name as a function of Item Number.
  With Application.WorksheetFunction
    ITEM_NAME = .Index(rng_ItemName, .Match(varItemNumber, rng_ItemNumber))
  End With
End Function

The cost, of course, is the overhead of a function call.


If you are planning on using the ListObject class for the final design, then why not use it now, and also use dynamic named ranges (the hard-coded ranges in the example are there so it actually works as is - these should be replaced with the named ranges):

Option Explicit

Private Function str_Table1() As String
    Static sstrTable1 As String
    If sstrTable1 = vbNullString Then
      sstrTable1 = Sheet1.Range("A4:B6").ListObject.Name
    End If
    str_Table1 = sstrTable1
End Function
Private Function str_ItemNumber() As String
    Static sstrItemNumber As String
    If sstrItemNumber = vbNullString Then
      sstrItemNumber = Sheet1.Range("A4:A6").Offset(-1).Resize(1).Value2
    End If
    str_ItemNumber = sstrItemNumber
End Function
Private Function str_ItemName() As String
    Static sstrItemName As String
    If sstrItemName = vbNullString Then
      sstrItemName = Sheet1.Range("B4:B6").Offset(-1).Resize(1).Value2
    End If
    str_ItemName = sstrItemName
End Function

Public Function ITEM_NAME(varItemNumber As Variant) As String
  'Returns Item Name as a function of Item Number.
  Dim ƒ As WorksheetFunction: Set ƒ = WorksheetFunction
  With Sheet1.ListObjects(str_Table1)
    ITEM_NAME _
    = ƒ.Index _
      ( _
        .ListColumns(str_ItemName).DataBodyRange _
      , ƒ.Match(varItemNumber, .ListColumns(str_ItemNumber).DataBodyRange) _
      )
  End With
End Function

Once the logic/design is ready, you can replace the functions with module-level constants of the same name if speed is critical and you need to reclaim the function call overhead. Otherwise, you can just leave everything as is.

Note that the use of static variables is not required, but should reduce execution time. (Static variables could also have been used in the first example as well, but I left them out to keep it short.)

It's probably not really necessary to extract out the table names into pseudo-constants, but I have done so for completeness sake.


EDIT: (v2)

Following up on Egalth's two brilliant suggestions, leads to the follow code which obviates the need for named ranges, or even hard-coded cell addresses, altogether as we leverage the builtin dynamism of the ListObject table itself.

I have also changed the parameter name to match* the relevant column header name so when the user presses Ctrl+Shift+A a hint as to which column to use appears. (This tip and, if required, more info on how to add Intellisense tool-tips and/or get a description to appear in the Function Arguments dialog can be seen here.)

Option Explicit

Private Function str_Table1() As String
    Static sstrTable1 As String
    If sstrTable1 = vbNullString Then sstrTable1 = Sheet1.ListObjects(1).Name ' or .ListObjects("Table1").Name
    str_Table1 = sstrTable1
End Function
Private Function str_ItemNumber() As String
    Static sstrItemNumber As String
    If sstrItemNumber = vbNullString Then
      sstrItemNumber = Sheet1.ListObjects(str_Table1).HeaderRowRange(1).Value2
    End If
    str_ItemNumber = sstrItemNumber
End Function
Private Function str_ItemName() As String
    Static sstrItemName As String
    If sstrItemName = vbNullString Then
      sstrItemName = Sheet1.ListObjects(str_Table1).HeaderRowRange(2).Value2
    End If
    str_ItemName = sstrItemName
End Function

Public Function ITEM_NAME(ByRef Item_ID As Variant) As String
  'Returns Item Name as a function of Item Number.
  Dim ƒ As WorksheetFunction: Set ƒ = WorksheetFunction
  With Sheet1.ListObjects(str_Table1)
    ITEM_NAME _
    = ƒ.Index _
      ( _
        .ListColumns(str_ItemName).DataBodyRange _
      , ƒ.Match(Item_ID, .ListColumns(str_ItemNumber).DataBodyRange) _
      )
  End With
End Function

Note the usage of .Value2. I have always used .Value2 ever since I found out about the performance drag and other issues caused by the implicit type conversion done when using .Value (or when relying on it as the default property).

* Make sure to update the column header names in the code when the logic/design of the project is finished.


EDIT: (re-boot)

Re-reading your own comments to your posted Question, I noted this one:

I might adopt that approach eventually, but I'm still in the design process and moving columns around a lot so the index number might also change

Whilst the last example above allows the header names to be changed dynamically, moving/inserting columns changes the indexes, requiring the code to be modified.

Looks like we're back to using named ranges. However, this time we only need static ones pointing to the column headers.

It also turns out that, for this new case, static variables are a bad idea in the design stage. Since the column indexes are cached, inserting a new column breaks the UDF until the project is reset.

I have also incorporated a shortened version of the sheet-less table reference hack from the quote in your posted Question:

Option Explicit

Private Function str_Table1() As String
    str_Table1 = Sheet1.ListObjects(1).Name
End Function
Private Function str_ItemNumber() As String
    With Range(str_Table1).ListObject
      str_ItemNumber = .HeaderRowRange(.Parent.Range("A3").Column - .HeaderRowRange.Column + 1).Value2
    End With
End Function
Private Function str_ItemName() As String
    With Range(str_Table1).ListObject
      str_ItemName = .HeaderRowRange(.Parent.Range("B3").Column - .HeaderRowRange.Column + 1).Value2
    End With
End Function

Public Function ITEM_NAME(ByRef Item_ID As Variant) As String
  'Returns Item Name as a function of Item Number.
  Dim ƒ As WorksheetFunction: Set ƒ = WorksheetFunction
  With Range(str_Table1).ListObject
    ITEM_NAME _
    = ƒ.Index _
      ( _
        .ListColumns(str_ItemName).DataBodyRange _
      , ƒ.Match(Item_ID, .ListColumns(str_ItemNumber).DataBodyRange) _
      )
  End With
End Function

Note that you can't use Item_name for one of the named ranges as it is the same as the UDF (case is ignored). I suggest using a trailing underscore, eg, Item_name_, for your named ranges.


All the above methods would also have solved the original issue that you had. I'm awaiting the last pieces of info in order to make an educated guess as to why this issue was occurring in the first place.

robinCTS
  • 5,746
  • 14
  • 30
  • 37
  • Thanks for all this, interesting approach with pseudo-constants. But if that's indeed bad coding practice maybe I should just move all declarations to the scope of each UDF rather than using pseudo-constants. (And +1 for the elegant use of ƒ.) – Egalth Sep 09 '17 at 20:09
  • 1
    I'm not sure I follow the logic of the statement `sstrItemNumber = Sheet1.Range("A4:A6").Offset(...` Do you mean I should replace the hard-coded range with a name for that range? Or just skip the named ranges altogether and access the column headers "dynamically" with Offset in this way? (A simpler version of that I guess would be `Sheet1.Range("A4").Offset(-1).Value`, without having to resize.) – Egalth Sep 09 '17 at 20:15
  • @Egalth I should have been clearer. Using **global variables** is bad coding practice. *Unnecessary* use of **module-level variables** falls under the same category. It *may* be necessary in special cases to use module-level variables. (I haven't found one yet though.) This would probably be OK. After all, you *are* reducing the scope and they *do* have to be declared together at the top of the module. On the other hand, normalizing your code so that what are essentially calculated constants are abstracted away, and you avoid entering them multiple times, can only be considered a good thing… – robinCTS Sep 10 '17 at 04:47
  • 1
    Using functions to avoid "global" or "global-like" *variables* is precisely one of the correct techniques to employ. – robinCTS Sep 10 '17 at 04:47
  • 1
    @Egalth As far as the usage of ƒ goes, ***everywhere*** you read ***everyone*** says it's a very bad practice to use non-standard characters for variable names. I, however, use five for very specific and carefully thought out situations. See this recent answer [here](https://stackoverflow.com/a/46036068/1961728) for two of them. (Soon to be three in the next bug-fix.) I have been doing so since I discovered RVBA years ago and tweaked it into a form I call R³VBA. – robinCTS Sep 10 '17 at 04:47
  • 1
    @Egalth Following in the style of your example, I did mean for you to replace the hard-coded ranges with the dynamic named ranges you use in your real file. I *had* realized that you could avoid the resize by using the start cell of the column only, but with the notion stuck in my head that these hardcoded values where for illustration only and your named ranges where the actual usage, I completely missed your fantastic simplification. Getting rid of the named ranges altogether is brilliant! – robinCTS Sep 10 '17 at 04:47
  • 1
    For the sake of even greater dynamism let's make it independent of the Table's location on the sheet: `sheet1.listobjects(1).databodyrange.cells(1,1).offset(-1).value2` – Egalth Sep 10 '17 at 12:03
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/154113/discussion-between-robincts-and-egalth). – robinCTS Sep 11 '17 at 08:26