Rubberduck: Code inspection for ParamArray parameter getting passed as an argument to a method taking it as a ParamArray parameter

Created on 12 Dec 2018  路  12Comments  路  Source: rubberduck-vba/Rubberduck

I've just had a couple of head scratching hours due to defining a function with a parameter array in an interface.

To prevent compilation errors the class procedure also has to be declared with a paramarray. However the array received by the class procedure is consolidated into a single variant containing an array, rather than an array.

At the interface you have

Paramarray(0)
Paramarray(1)
Paramarray(2)... etc

at the class procedure you get instead

Paramarray(0)(0,0)
Paramarray(0)(0,1)
Paramarray(0)(0,2)...etc

If the class procedure is used internally as well as through the interface you end up with two different types of parameter list

internal: an array of variants

interface: a variant containing an array of variants

There doesn't appear to be an inspection warning for paramarrays in Interfaces. Could we please add one.

difficulty-02-ducky enhancement feature-inspections up-for-grabs

Most helpful comment

Not sure the interface is relevant. That's just a by-product of ParamArray being, well, a Variant array. A procedure with a ParamArray parameter should take steps to validate whether it's getting an array of values, or a single-element array containing an array of values. Any method passing a ParamArray argument to another method taking a ParamArray parameter, will be passing it as a single-element variant array holding the array of values.

The inspection should be about a ParamArray parameter being passed as an argument to a method taking a ParamArray parameter: the called procedure will find the original arguments in args(0).

Interfaces have nothing to do with this.

All 12 comments

To be clear, are you calling the class procedure from the interface?

If that is the case, then the workaround would be to define the class procedure as taking a Variant rather than an ParamArray. If the class procedure is public/friend, then both the class procedure and the interface procedure should call an internal procedure that takes a Variant rather than ParamArray.

Agreed about an inspection but need to think about how it's going to be handled.

Not sure the interface is relevant. That's just a by-product of ParamArray being, well, a Variant array. A procedure with a ParamArray parameter should take steps to validate whether it's getting an array of values, or a single-element array containing an array of values. Any method passing a ParamArray argument to another method taking a ParamArray parameter, will be passing it as a single-element variant array holding the array of values.

The inspection should be about a ParamArray parameter being passed as an argument to a method taking a ParamArray parameter: the called procedure will find the original arguments in args(0).

Interfaces have nothing to do with this.

In the interface

Public Sub AddValuesItems(ByVal property_path As String, ParamArray Items() As Variant)

In the class

Private Sub ICxpSimplifier_AddValuesItems(ByVal property_path As String, ParamArray Items() As Variant)
     AddValuesItems this_property_path ,items
End sub

and

Public Sub AddValuesItems(ByVal property_path As String, ParamArray Items() as Variant)

ParamArray array is passed as a ParamArray argument
An array of parameter values ('ParamArray') is being passed as the first element of a 'ParamArray' argument: the invoked procedure will recceive the variant array at the base index of its own parameter array. Procedures taking a 'ParamArray' argument should take steps to verify whether the only element of the array contains another array.

Okay that's tentative wording - and definitely needs some work. But the triggering code would be this:

Public Sub DoSomething(ParamArray foo() As Variant)
    Dim i As Long
    For i = LBound(foo) To UBound(foo)
        ' assume foo(i) is a parameter
    Next
End Sub

And, ideally, this wouldn't trigger the inspection:

Public Sub DoSomething(ParamArray foo() As Variant)
    Dim values As Variant
    If LBound(foo) >= 0 And UBound(foo) = LBound(foo) And IsArray(foo(LBound(foo))) Then
        'foo has 1 value; foo(LBound(foo)) has the parameters
        values = foo(LBound(foo))
    Else
        'foo has the parameters
        values = foo
    End If
    Dim i As Long
    For i = LBound(values) To UBound(values)
        ' ...
    Next
End Sub

Avoiding false positives will be "fun" here....

@SteveLaycock that's what I suspected. It's not really to do with the interfaces as @retailcoder pointed out. See my prior comment for workarounds in that case since you are calling a class implementation of the function from the class' implementation of the interface.

Its the implication that is the problem. If I pass a list of variables via an interface I expect the class to receive the same list of variables. That's whatcaught me out. If I were passing a paramarray to another procedure I would know exactly what to expect.

@SteveLaycock sounds like your interface implementation is invoking some equivalent procedure on the class' default public interface, so you're passing a ParamArray parameter as a ParamArray argument, correct?

e.g.

Implements Thing

Public Sub DoStuff(ParamArray stuff() As Variant)
    Thing_DoStuff stuff
End Sub

Public Sub Thing_DoStuff(ParamArray stuff() As Variant)
'here stuff is only 1 element when invoked from Me.DoStuff
End Sub

If that's the case, then the fact that an interface is involved, is irrelevant to the issue - this has everything to do with how ParamArray works... and I agree it warrants an inspection result somehow.

Well yes and no. I understand how paramarray works and I don't have a problem with it. However I mistakenly assumed that the interface would be transparent and not mutate the structure of of the procedure parameter list. Its what happens when you are ' Clever enough to understand you're not clever enough'

I think one reason why a ParamArray parameter on a public member is problematic, especially on an interface, is that you cannot write a decorator for such class. Thus, the natural way to emulate implementation inheritance on in VBA is barred, unless there is a method that takes an explicit array and mirrors the method with the ParamArray.

I know it's only tangential to the problem discussed... but I had very much the same issue one time and came up with the following function:

' Return the actual parameters from nested ParamArrays
Private Function UnwrapParamArray(ParamArray arrayToUnwrap() As Variant) As Variant()
    Dim OutputArray() As Variant
    Dim IntermediateArray() As Variant
    If UBound(arrayToUnwrap) > 0 Then
        OutputArray = arrayToUnwrap
        Exit Function
    End If
    OutputArray = arrayToUnwrap(0)
    Do While (UBound(OutputArray) = 0) And (TypeName(OutputArray(0)) = "Variant()")
        IntermediateArray = OutputArray(0)
        OutputArray = IntermediateArray
        If UBound(OutputArray) = -1 Then Exit Do
    Loop
    UnwrapParamArray = OutputArray
End Function

(It's been a while and since I neglected commenting, I can't tell for sure that everything in there is actually needed. What I can say though, is that it works fine for me. I'm using it to flatten nested arrays in my logger classes.)
The function can be used everywhere you might encounter nested ParamArrays. (As it just returns the array unchanged if it's already flat.)

If that is the case, then the workaround would be to define the class procedure as taking a Variant rather than an ParamArray. If the class procedure is public/friend, then both the class procedure and the interface procedure should call an internal procedure that takes a Variant rather than ParamArray.

This is a much easier and cleaner approach. The minor difference is that instead of having a list of items in the parameter list, the list of items is enclosed in an Array() statement.

I can definitely see a change signature refactoring that, when changing a ParamArray to a Variant, changes the call sites from foo, bar, 42 to Array(foo, bar, 42).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

SteGriff picture SteGriff  路  3Comments

retailcoder picture retailcoder  路  3Comments

ghost picture ghost  路  3Comments

connerk picture connerk  路  3Comments

Gener4tor picture Gener4tor  路  3Comments