0

First time posting, apologies if I make any mistakes!

So, I'm having a pretty strange problem with my UDF. In my workbook, I have an invisible 'template' sheet named "Standard Phase Sheet", and a subroutine that a user can activate which copies that template sheet into a new, visible sheet that the user can then work with. There will be many copies of that template sheet throughout the workbook, but they will all have unique names.

My UDF is on that template sheet in several spots, and thus on every copy of the template sheet that a user makes. When working within one of these sheets, the UDF works just fine, and returns the values I'd expect.

However, when a user ADDS a new copy of the template sheet, SOMETIMES the UDF goes haywire and returns #VALUE errors in every place the UDF is being used.

Also, when a user DELETES one of the copies of the template sheet, the UDF ALWAYS goes haywire and returns #VALUE errors in every place the UDF is being used.

I'm not using ActiveSheet or anything like that, and I believe I'm correctly giving full references to the ranges I'm working with within the UDF. Any help will be appreciated, I'm in a bind here! Code for the UDF is below.

Also, because I'm sure I'll be asked the question, the neColumn variable within my code is a public variable that I use in several subroutines and UDFs. It is defined at the beginning of my module. Also, I am using Option Explicit at the beginning of my module as well.

Thank you!

Public Function fSum(ByVal Target As Range, bExtended As Boolean) As Single

'This function returns a sum, based on a range provided in the cell that holds the function.
'It checks to see if that line item has been marked as Non-Extended, based on the NE column
'that can be check marked. If that line item is marked NE, then only the NE sum columns can
'use that line item as part of their sum, and those values are removed from the E columns.

Dim sSum As Single
Dim i As Integer
Dim n As Integer

'This small section is used to determine complete references to the cell calling the function.

Dim sheetName As String
sheetName = Application.Caller.Parent.Name

'Loop through provided range, and sum up the contents based on whether they have been marked NE or not.

i = 1
n = Target.row
sSum = 0

If Sheets(sheetName).Visible = True Then

While i < Target.Rows.Count

    If (bExtended = True) Then

        If Sheets(sheetName).Range(neColumn.Address).Cells(n, 1) = vbNullString Then

            sSum = sSum + Sheets(sheetName).Range(Target.Address).Cells(i, 1).Value

        End If

    Else

        If Sheets(sheetName).Range(neColumn.Address).Cells(n, 1) <> vbNullString Then

            sSum = sSum + Sheets(sheetName).Range(Target.Address).Cells(i, 1).Value

        End If

    End If

i = i + 1
n = n + 1

Wend

End If

fSum = sSum

End Function
larryengineering
  • 3
  • 3
  • 1
    Side note: UDF's can't (except in some edge cases) modify the sheet. So the line `Sheets(sheetName).Range(neColumn.Address).Cells(n, 1).Font.Name = "Marlett"` shouldn't be in a UDF. – BigBen Jan 29 '20 at 13:56
  • You have `sSum = sSum + Sheets(sheetName).Range(Target.Address).Cells(i, 1).Value` within two distinct `If` blocks - that means the `= vbNullString` and `<> vbNullString` checks are not needed, because you do the same thing either way. – BigBen Jan 29 '20 at 14:01
  • You're right, thanks for the comment. I've edited the code in the original question to reflect the line that was attempting to change the font of the cell. But regarding the nested `if`blocks, I'm not sure that's correct. The `= vbNullString` and `<> vbNullString` are only evaluated whether `bExtended` is true or not, respectively, so the code doesn't do the same thing either way. – larryengineering Jan 29 '20 at 14:09
  • Good point on the `If` logic. Other notes: `While...Wend` is obsolete - prefer `Do While ... Loop`, and I don't see the point of the `Visible` check - who cares whether the UDF is evaluted on a hidden sheet? – BigBen Jan 29 '20 at 14:12
  • 1
    Also, `Long` should be used instead of `Integer`. More context [here](https://stackoverflow.com/questions/26409117/why-use-integer-instead-of-long). – BigBen Jan 29 '20 at 14:13
  • Regarding the visible check, it was just an attempt by me to fix the question I asked about. In my attempts at debugging, I noticed it was being run on the template sheet, and even the newly copied sheet before it was visible. I didn't think it would solve my issue, but it was worth a shot. So yeah, it's not really needed anymore. However, can you explain the issue with using `Integer`, and also with using `While...Wend`? I feel like those really wouldn't have anything to do with my root issue. – larryengineering Jan 29 '20 at 14:17
  • It honestly feels like this UDF is replicating SUMIFS functionality. Or maybe SUMPRODUCT. – BigBen Jan 29 '20 at 14:18
  • Well....this is embarrassing! I believe in this case, I'll be able to get away with SUMIFS. There's a slightly more complex version of my UDF though that I use elsewhere in the same sheet, but I may be able to use SUMIFS there as well, not sure yet. I'll go test it out in both applications and get back to you, thanks! For context, this is an old workbook I've been tasked with updating, and I'm not sure when SUMIFS was even introduced to Excel. Maybe that's why the original author of the code made their own UDF. So I got wrapped up in their code, and didn't even consider using a sheet function. – larryengineering Jan 29 '20 at 14:30
  • Eh - no problem, no embarrassment either. All that matters is that your problem gets resolved :) – BigBen Jan 29 '20 at 14:32
  • Yup, that solved the issue, in both cases! My workbook now functions without any of my initial problems. I'd still love to know what was causing the problem in the first place, but I'm just happy it works now. BigBen, submit your suggestion as an answer and I'll mark it as accepted. Thank you very much! – larryengineering Jan 29 '20 at 15:52

2 Answers2

1

Summarizing the comment thread in an answer for posterity:

  • I'm not sure why exactly you see this behavior.
  • There would be ways to better this UDF (including using Long instead of Integer, preferring a Do While...Loop to While...Wend, removing the .Visible check...
  • But in any case, it does feel like this is just replicating the functionality of SUMIFS so you might just consider going that route.
BigBen
  • 46,229
  • 7
  • 24
  • 40
1

The reason is that your neColumn variable has become Nothing, because Excel is Volatile.

I assume that the start of your module looks something like this:

Option Explicit

Public neColumn As Range

Sub Auto_Open()
    Set neColumn = Sheet1.Range("A1:B2")
End Sub

When you open the Workbook, you call the Auto_Open Sub to Set the neColumn variable. However - when certain actions occur, Excel rebuilds the VBA, which resets the Public Variables (such as neColumn) to their defaults (which, for an Object such as a Range, is Nothing). An easy way to trigger this is by deliberately throwing an error, such as attempting to run this:

Sub ThrowErr()
    NotDefined = 1
End Sub

You can make it more visible to you by adding the following line to your fSum code:

If neColumn Is Nothing Then Stop

You either need a way to restore neColumn when it has been reset to Nothing, OR find a non-volatile way to store it.

I am assuming that this is not suitable to become a Const, because otherwise it already would be but you could turn it into a Named Range, or store the Address in a hidden worksheet / CustomDocumentProperty. These options would also allow you to store neColumn when the Workbook is saved for when you reopen it

Chronocidal
  • 6,827
  • 1
  • 12
  • 26
  • Yep this was my guess as well. Why `Public` ranges are a bad idea. – BigBen Jan 29 '20 at 17:20
  • I didn't know any of those specifics, but it definitely sounds like the cause of my struggles. I haven't run into any issues using the variable elsewhere in my module/sheets, but I'll keep this information in mind if I do. Thanks for the input! – larryengineering Jan 29 '20 at 17:29