C++ Reduce Redundancy

Question

I have many C++ functions in a DLL that I made specifically for Excel to call.

I frequently pass to these functions as parameters OLE VARIANTs that contain SafeArrays.

I wrote a function to act as a guard to ensure that a passed VARIANT actually contains a SafeArray and that the array is of the proper type for the occasion, both in terms of datatype and number of dimensions.

If those three criteria are satisfied the function returns a pointer to the first element and also has two out parameters to return the number of elements in each dimension (note: I only care about 1d and 2d SafeArrays).

Here is the function:

PVOID SafeArrayDataPointer(VARIANT &v, const long &ArrayType, const long &ArrayDims, long &Elems1D, long &Elems2D) {

    SAFEARRAY* pSafeArray = NULL;

    if ( V_VT(&v) & VT_ARRAY ) {

        if ( ArrayType != (V_VT(&v) & VT_TYPEMASK) ) return NULL;

        pSafeArray = V_ARRAY(&v);
        if ( ArrayDims != pSafeArray->cDims ) return NULL;

        switch (ArrayDims) {
        case 2:
            Elems1D = (pSafeArray->rgsabound)[1].cElements;
            Elems2D = (pSafeArray->rgsabound)[0].cElements;
            break;
        case 1:
            Elems1D = (pSafeArray->rgsabound)[0].cElements;
            Elems2D = 0;
            break;
        default: 
            Elems1D = 0;
            Elems2D = 0;
            break;
        }

        return pSafeArray->pvData;

    } else return NULL;

}

This function works well and allows me to conveniently grab the data pointer and to get the number of elements in each dimension by calling like this (assuming vData is a VARIANT passed from Excel:

pDataArray = (VARIANT*) SafeArrayDataPointer(vData, VT_VARIANT, 2, Elems1D, Elems2D); if (pDataArray == NULL) goto error1;

However, there are two things that I do not like about this:

1.) Since the function returns a PVOID I have to cast to the relevant pointer type... but this is redundant as I've already specfied in the second argument what type of array must be contained in the VARIANT. For example in a different situation I may need to make sure that the array is of long values:

pDataArray = (long*) SafeArrayDataPointer(vData, VT_I4, 1, Elems1D, Junk); if (pDataArray == NULL) goto error1;

Again, this works fine, but it is redundant. I would much prefer some mechanism that allows the function to return the correct type of pointer. And I'm hoping this can be done without templates.

2.) I can't figure out how to NOT have the Junk parameter in the second example where I specify that the array must be 1d. Obviously, in such a case, there are no elements in the second dimension.

Do you not want to create multiple functions: `SafeArrayVariantPointer`, `SafeArrayLongPointer`, ... etc? Would you be okay with using a gross macro? You could also wrap the goto on error condition inside this gross macro as well. — MFisherKDX, Mar 28 '19 at 18:27
Could you demo the macro? But I really don't want a bunch of different functions for different datatype's. That would be even much more redundant than what I'm currently doing. — Excel Hero, Mar 28 '19 at 18:29
Regarding the `Junk` parameter, why can't you make a second version of the `SafeArrayVariantPointer` that only returns the size of the first dimension? It can call the first version with a Junk parameter and even assert that it's 0. — MFisherKDX, Mar 28 '19 at 18:29
ok. i'll post something on the macro. it is gross and i expect people to downvote it. but it should keep you from repeating your casts at every function call. — MFisherKDX, Mar 28 '19 at 18:37

MFisherKDX · Accepted Answer · 2019-03-29T16:16:54.793

If you don't want to use templates, and you don't want specialized functions, you could use a gross macro to solve the first problem about having to cast from PVOID every time you call SafeArrayVariantPointer:

Something like this:

#define CAST_TYPE_VT_VARIANT (VARIANT *)
#define CAST_TYPE_VT_LONG (long *)

#define SAFE_ARRAY_DATA_POINTER(vData, vType, dimSize, elems1D, elems2D) \
    CAST_TYPE_##vType SafeArrayDataPointer((vData), (vType), (dimSize), (elems1D), (elems2D))

Then you can call like:

VARIANT *pDataArray = SAFE_ARRAY_DATA_POINTER(vData, VT_VARIANT, 2, Elems1D, Elems2D);

But first you need to change your method signature so the ArrayType argument is taken as a long not a const long &.

Note this assumes the second parameter to SAFE_ARRAY_DATA_POINTER macro must be a literal that corresponds to one of your defined CAST_TYPE_* macros. It can't be a variable.

For you second question about the redundant Junk parameter, you can create an overloaded SafeArrayDataPointer function which only returns the size of first dimension. It can call the first version of SafeArrayDataPointer and discard the size of the second dimension.

Something like:

PVOID SafeArrayDataPointer(VARIANT &v, long ArrayType, const long &ArrayDims, long &Elems1D) 
{
    long Elems2D;
    PVOID *toReturn = SafeArrayDataPointer(v, ArrayType, ArrayDims, Elems1D, Elems2D);
    if (Elems2D != 0) toReturn = NULL;
    return toReturn;
}

However, to solve this problem, I would probably use templates.

First, create a set of array_type_traits classes which expose a typedef for your cast type given a long representing VT_LONG, VT_VARIANT, etc.

//Generic array_type_traits class
template<long array_type>
class array_type_traits 
{
public:
    typedef PVOID cast_type;
};

//Specialized for VT_LONG
template<>
class array_type_traits<VT_LONG>
{
public:
    typedef long * cast_type; 
};

//Specialized for VT_VARIANT
template<>
class array_type_traits<VT_VARIANT>
{
public:
    typedef VARIANT * cast_type;
};

Continue to specialize these for each VT_* type you have.

Next, encapsulate your SafeArrayDataPointer function inside a class SafeArrayDataPointerBase.

//Base class which has one static function Get() that returns a PVOID
class SafeArrayDataPointerBase
{
protected:
    static PVOID Get(VARIANT& vData, long vType, long dimSize, long& elems1D, long& elems2D)
    {
        // Place your SafeArrayDataPointer function code here 
    }
};

Now create your class which will call `SafeArrayDataPointerBase::Get() and then cast the result to the correct type.

template<long ArrayType>
class SafeArrayDataPointer : public SafeArrayDataPointerBase
{
public:
    typedef typename array_type_traits<ArrayType>::cast_type cast_type;

    static cast_type Get(VARIANT& v, long ArrayDims, long& Elems1D, long& Elems2D)
    {
        return (cast_type) SafeArrayDataPointerBase::Get(v, ArrayDims, ArrayType, Elems1D, Elems2D);
    }
};

And finally, you would call the template class as so:

VARIANT *vp = SafeArrayDataPointer<VT_VARIANT>::Get(v, ArrayDims, Elems1D, Elems2D); 
long *vl = SafeArrayDataPointer<VT_LONG>::Get(v, ArrayDims, Elems1D, Elems2D);

This seems much more streamlined than the template version from C.M. Why do you not like this? — Excel Hero, Mar 29 '19 at 04:47
I would say macros are usually harder to debug than functions. Here is a link that explains the [differences between macros and functions](https://stackoverflow.com/questions/9104568/macro-vs-function-in-c). I think this macro solution would work for your use case although I would probably use templates myself. — MFisherKDX, Mar 29 '19 at 05:35
@ExcelHero - I just updated to show how I would do this with templates. compiled with g++ 7.2.0 — MFisherKDX, Mar 29 '19 at 16:17

C.M. · Answer 2 · 2019-03-28T19:11:14.707

Smth like this:

template<int vt> struct vt_type_disp;
template<> struct vt_type_disp<VT_I4> { typedef LONG type; };
template<> struct vt_type_disp<VT_VARIANT> { typedef VARIANT type; };

template<int vt> using vt_type = typename vt_type_disp<vt>::type;


template<int vtElem>
auto unpack_array(VARIANT& v, ULONG& d1_size)
{
    if ((V_VT(&v) & VT_ARRAY) && (V_VT(&v) & VT_TYPEMASK) == vtElem)
    {
        SAFEARRAY* p = (V_VT(&v) & VT_BYREF) ? *V_ARRAYREF(&v) : V_ARRAY(&v);

        if (p->cDims == 1)
        {
            d1_size = p->rgsabound[0].cElements;
            return static_cast<vt_type<vtElem>*>(p->pvData);
        }
    }
    return static_cast<vt_type<vtElem>*>(nullptr);
}

template<int vtElem>
auto unpack_array(VARIANT& v, ULONG& d1_size, ULONG& d2_size)
{
    if ((V_VT(&v) & VT_ARRAY) && (V_VT(&v) & VT_TYPEMASK) == vtElem)
    {
        SAFEARRAY* p = (V_VT(&v) & VT_BYREF) ? *V_ARRAYREF(&v) : V_ARRAY(&v);

        if (p->cDims == 2)
        {
            d1_size = p->rgsabound[1].cElements;
            d2_size = p->rgsabound[0].cElements;
            return static_cast<vt_type<vtElem>*>(p->pvData);
        }
    }
    return static_cast<vt_type<vtElem>*>(nullptr);
}

// functions to export from dll (if you need them)
auto unpack_array_I4_1D(VARIANT &v, ULONG& dim_size) { return unpack_array<VT_I4>(v, dim_size); }
auto unpack_array_I4_2D(VARIANT &v, ULONG& d1_size, ULONG& d2_size) { return unpack_array<VT_I4>(v, d1_size, d2_size); }
auto unpack_array_VARIANT_1D(VARIANT &v, ULONG& dim_size) { return unpack_array<VT_VARIANT>(v, dim_size); }
auto unpack_array_VARIANT_2D(VARIANT &v, ULONG& d1_size, ULONG& d2_size) { return unpack_array<VT_VARIANT>(v, d1_size, d2_size); }
// etc

Note: your code doesn't handle VT_BYREF properly

@MFisherKDX he said "I'm hoping this can be done without templates"... These templates can be "unwrapped" into few macros anyway. — C.M., Mar 28 '19 at 19:12

C++ Reduce Redundancy

2 Answers2