0

In a sub I've made in VBA, which is executed everytime the worksheet changes, the sub calls a function to determine if the user has deleted or changed something they shouldn't have. The function returns a boolean value to tell the sub if such a critical value has been deleted/edited. This way, the sub knows not to continue executing code.

However, whenever the code executes, VBA returns a Compile Error: Expected array error for the calling of the criticalDataIntact() function, despite there being no arrays used.

Here's my relevant code,

Private Sub Worksheet_Change(ByVal target As Range)
    Worksheets(CONFIG).usedRange 'Refresh UsedRange
    Dim criticalDataIntact As Boolean: criticalDataIntact = criticalDataIntact()

    If Not criticalDataIntact Then
        Exit Sub
    End If

    'Irrelevant code
End Sub

Private Function criticalDataIntact() As Boolean
    Dim criticalDataIntact As Boolean: criticalDataIntact = True
    Set warnWorkloadCell = ThisWorkbook.cell(CONFIG, WARNING_WORKLOAD, 0, 0)
    Set dangerWorkloadCell = ThisWorkbook.cell(CONFIG, DANGER_WORKLOAD, 0, 0)
    Dim table3Exists As Boolean: table3Exists = tableExists("Table3")

    If warnWorkloadCell = Null Or dangerWorkloadCell = Null Or table3Exists = False Then
        criticalDataIntact = False
    End If
End Function

The cell function outputs an error message via MsgBox and returns Null if it couldn't find a cell with a specific value in it within a specific worksheet.

Any ideas?

HarrisonO
  • 147
  • 1
  • 8
  • 2
    Why are you declaring a variable the same name as the function it's located in? – K.Dᴀᴠɪs Jul 13 '19 at 08:14
  • @K.Davis isn't that how returning a value works in VBA? I tried using the `Return` keyword, but that got a `Expected: end of statement` error. According to Dan on https://stackoverflow.com/questions/2781689/how-to-return-a-result-from-a-vba-function, the way I've done it is the correct way. – HarrisonO Jul 13 '19 at 09:31
  • No. You have already declared the return type with signature criticalDataIntact() As Boolean. If you want a local boolean variable inside the function use a different name then at end have criticalDataIntact = localBoolean.... But I think you can simply the above into criticalDataIntact = not (c1 = Null Or c2 = Null Or c3 = False) and no need for loca variable – QHarr Jul 13 '19 at 10:00
  • Supplementing QHarr's comment: VBA does not use `return` to pass function results back to the calling procedure. That's a ,NET framework way to do things (VB.NET). Instead, you set the name of the function to the result to be passed back (returned) at the end of the function code. In contrast to QHarr's last remark I prefer/recommend a local variable and passing back at the end. That's clearer and less prone to errors (in my opinion). – Cindy Meister Jul 13 '19 at 10:27

2 Answers2

2
  • Consider renaming your CriticalDataIntact function to IsCriticalDataIntact.
  • If your Worksheets(CONFIG) worksheet is the same worksheet containing the Worksheet_Change code, consider replacing Worksheets(CONFIG) with Me.
  • If you don't refer to your criticalDataIntact variable anywhere else in your Worksheet_Change subroutine (i.e. anywhere in the "irrelevant code"), consider removing the variable and just using the return value directly.

Based on the above, your Worksheet_Change routine might look something like:

Private Sub Worksheet_Change(ByVal target As Range)
    Worksheets(CONFIG).UsedRange ' Doesn't this give you a syntax error? If this is the same worksheet as the worksheet storing this procedure, consider using Me.UsedRange -- although maybe there a better alternatives to UsedRange.

    If Not IsCriticalDataIntact() Then
        Exit Sub
    End If

    'Irrelevant code
End Sub
  • Consider Option Explicit at the top of the module. I say this because your CriticalDataIntact function contains seemingly undeclared variables: warnWorkloadCell, dangerWorkloadCell (unless these are non-local variables).

  • I don't have access to your custom functioncell, but presume it is a member of ThisWorkbook for you. Consider changing the name cell to something more descriptive

  • Regarding cell function's return type, you've said it returns either a Range or Null. That sounds like Variant behaviour (my understanding is only Variants can store Null in VBA). But I think this will lead to invalid syntax at the call site, since you can't Set a Variant to Null (as far as I know).
  • Sounds like what you want is for the return type to be Range -- and if a Range can't be returned, Nothing will be returned (which you can still test for).
  • Based on the above, a better name for your custom function cell might be GetCellOrNothing.

So your function might look something like:

Private Function IsCriticalDataIntact() As Boolean

    Dim warnWorkloadCell As Range ' Declare if a local variable.
    Set warnWorkloadCell = ThisWorkbook.cell(CONFIG, WARNING_WORKLOAD, 0, 0) ' Consider renaming to "GetCellOrNothing" -- depending on how much work that is for you.

    Dim dangerWorkloadCell As Range ' Declare if a local variable.
    Set dangerWorkloadCell = ThisWorkbook.cell(CONFIG, DANGER_WORKLOAD, 0, 0) ' Consider renaming to "GetCellOrNothing" -- depending on how much work that is for you.

    Dim table3Exists As Boolean
    table3Exists = tableExists("Table3")

    IsCriticalDataIntact = table3Exists And Not ((warnWorkloadCell Is Nothing) Or (dangerWorkloadCell Is Nothing)) 

End Function

Untested.

chillin
  • 4,391
  • 1
  • 8
  • 8
0

Remove the () after criticalDataIntact otherwise you have said criticalDataIntact = variant variable as the criticalDataIntact() is an implicit variant variable with name criticalDataIntact. That is the source of your error.

Dim criticalDataIntact As Boolean: cIntact = criticalDataIntact '<remove the () which is implicit variant

Then to avoid confusing compiler, use a differently named variable to hold the return value

Dim cIntact As Boolean: cIntact = criticalDataIntact

This

If warnWorkloadCell = Null Or dangerWorkloadCell = Null Or table3Exists = False Then
    criticalDataIntact = False
End If

Can become

criticalDataIntact = Not (IsNull(warnWorkloadCell) Or IsNull(dangerWorkloadCell) Or table3Exists = False)

and remove need for local boolean. Personally, I would change table3Exists to tableNotExists and make necessary changes inside that function so I could do

criticalDataIntact = Not (IsNull(warnWorkloadCell) Or IsNull(dangerWorkloadCell) Or table3NotExists)

I can't see your cell method so I don't know what it is returning but for objects I would do a If Not x Is Nothing normally or If x Is Nothing.

QHarr
  • 83,427
  • 12
  • 54
  • 101