Can these methods that convert safe arrays into std::list objects be turned into a template function?

Question

Take these three methods that each build distinct std::list objects from safe arrays:

void CMSATools::ConvertSAFEARRAY_DISCUSSIONITEMS(SAFEARRAY* psaDiscussionItems, ListDiscussionItems& rListDiscussionItems)
{
    MSAToolsLibrary::IDiscussionItemPtr* pVals = nullptr;
    HRESULT hr = SafeArrayAccessData(psaDiscussionItems, (void**)&pVals); // direct access to SA memory

    if (SUCCEEDED(hr))
    {
        long lowerBound, upperBound;  // get array bounds
        hr = SafeArrayGetLBound(psaDiscussionItems, 1, &lowerBound);
        if (FAILED(hr))
            throw _com_error(hr);

        hr = SafeArrayGetUBound(psaDiscussionItems, 1, &upperBound);
        if (FAILED(hr))
            throw _com_error(hr);

        rListDiscussionItems.clear();
        long cnt_elements = upperBound - lowerBound + 1;
        for (int i = 0; i < cnt_elements; ++i)  // iterate through returned values
        {
            rListDiscussionItems.push_back(pVals[i]);
        }
        hr = SafeArrayUnaccessData(psaDiscussionItems);
        if (FAILED(hr))
            throw _com_error(hr);
    }
    else
    {
        throw _com_error(hr);
    }

    hr = SafeArrayDestroy(psaDiscussionItems);
    if (FAILED(hr))
        throw _com_error(hr);
}

void CMSATools::ConvertSAFEARRAY_STUDENTITEMS(SAFEARRAY* psaStudentItems, ListStudentItems& rListStudentItems)
{
    MSAToolsLibrary::IStudentItemPtr    *pVals = nullptr;
    HRESULT hr = SafeArrayAccessData(psaStudentItems, (void**)&pVals); // direct access to SA memory

    if (SUCCEEDED(hr))
    {
        long lowerBound, upperBound;  // get array bounds
        hr = SafeArrayGetLBound(psaStudentItems, 1, &lowerBound);
        if (FAILED(hr))
            throw _com_error(hr);

        hr = SafeArrayGetUBound(psaStudentItems, 1, &upperBound);
        if (FAILED(hr))
            throw _com_error(hr);

        rListStudentItems.clear();
        long cnt_elements = upperBound - lowerBound + 1;
        for (int i = 0; i < cnt_elements; ++i)  // iterate through returned values
        {
            rListStudentItems.push_back(pVals[i]);
        }
        hr = SafeArrayUnaccessData(psaStudentItems);
        if (FAILED(hr))
            throw _com_error(hr);
    }
    else
    {
        throw _com_error(hr);
    }

    hr = SafeArrayDestroy(psaStudentItems);
    if (FAILED(hr))
        throw _com_error(hr);
}

void CMSATools::ConvertSAFEARRAY_DUTYHISTORYITEMS(SAFEARRAY* psaHistoryItems, ListDutyHistoryLookupItems& rListHistoryItems)
{
    MSAToolsLibrary::IDutyAssignmentLookupPtr   *pVals = nullptr;
    HRESULT hr = SafeArrayAccessData(psaHistoryItems, (void**)&pVals); // direct access to SA memory

    if (SUCCEEDED(hr))
    {
        long lowerBound, upperBound;  // get array bounds
        hr = SafeArrayGetLBound(psaHistoryItems, 1, &lowerBound);
        if (FAILED(hr))
            throw _com_error(hr);

        hr = SafeArrayGetUBound(psaHistoryItems, 1, &upperBound);
        if (FAILED(hr))
            throw _com_error(hr);

        rListHistoryItems.clear();
        long cnt_elements = upperBound - lowerBound + 1;
        for (int i = 0; i < cnt_elements; ++i)  // iterate through returned values
        {
            rListHistoryItems.push_back(pVals[i]);
        }
        hr = SafeArrayUnaccessData(psaHistoryItems);
        if (FAILED(hr))
            throw _com_error(hr);
    }
    else
    {
        throw _com_error(hr);
    }

    hr = SafeArrayDestroy(psaHistoryItems);
    if (FAILED(hr))
        throw _com_error(hr);
}

They all work and are functional. But they have a lot of common aspects. It is possible to use "templates" here? It is not something I have done before.

I don't mind having three distinct methods but if they could perhaps call one common templated method that converts the safe array it would make the code maintenance easier.

Make sense?

Answer 1

Yes, it looks like the bodies of these functions are similar enough that it would be fairly easy to collapse them all down to a single function template.

But no, I don't think that's really the right way to go here. Instead, I'd create a little proxy class to let a SAFEARRAY act like a range.

#include <windows.h>
#include <oleauto.h>


template <class F, class ...Args>
void check(F f, Args ...args) {
    auto hr = f(args...);
    if (FAILED(hr))
        throw hr;
}

template <class T>
class SafeArrayRange {
    SAFEARRAY *data;
    T* pVals {nullptr};
    long count;
public:
    using value_type = T;
    using reference = T&;
    using pointer = T*;
    using difference_type = std::ptrdiff_t;
    using iterator_tag = std::random_access_iterator_tag;

    SafeArrayRange(SAFEARRAY *array) : data(array) {
        check(SafeArrayAccessData, data, (void**)&pVals);
        long lowerBound;
        check(SafeArrayGetLBound, data, 1, &lowerBound);
        long upperBound;
        check(SafeArrayGetUBound, data, 1, &upperBound);
        count = upperBound - lowerBound + 1;
    }

    using iterator = T*;

    iterator begin() const { return pVals; }
    iterator end() const { return pVals + count;  }
    std::size_t size() const { return count; }
    ~SafeArrayRange() { SafeArrayUnaccessData(data); }
};

With that, I can treat a SAFEARRAY about like I would a vector or other normal container. For example, I did at least a minimal test of the range adapter using the following code:

int main() {
    SAFEARRAY* array = SafeArrayCreateVector(VT_I4, 0, 20);

    SafeArrayRange<uint32_t> range(array);

    // put data into the array:
    std::iota(range.begin(), range.end(), 0);

    // check that our idea of its size matches what we specified:
    std::cout << "range size: " << range.size() << "\n";

    std::vector<uint32_t> result;

    // get data back out of the array:
    std::copy(range.begin(), range.end(), std::back_inserter(result));

    // print out the contents of the array:
    std::copy(range.begin(), range.end(), std::ostream_iterator<uint32_t>(std::cout, "\t"));
}

So, I'd put the range adapter into a header and include it where needed, so most of the time I could kind of ignore most of the internal details of SAFEARRAYs. Warning: this is only minimally tested, so there may be a few bits and pieces here and there that I've missed.

Answer 2

Try this (didn't compile it):

template<typename from, typename to>
void CMSATools::ConvertSAFEARRAY_DUTYHISTORYITEMS(SAFEARRAY* psaItems, to& rItems)
{
  from   *pVals = nullptr;
  HRESULT hr = SafeArrayAccessData(psaItems, (void**)&pVals); // direct access to SA memory

  if (SUCCEEDED(hr))
  {
    long lowerBound, upperBound;  // get array bounds
    hr = SafeArrayGetLBound(psaItems, 1, &lowerBound);
    if (FAILED(hr))
      throw _com_error(hr);

    hr = SafeArrayGetUBound(psaItems, 1, &upperBound);
    if (FAILED(hr))
      throw _com_error(hr);

    rItems.clear();
    long cnt_elements = upperBound - lowerBound + 1;
    for (int i = 0; i < cnt_elements; ++i)  // iterate through returned values
    {
      rItems.push_back(pVals[i]);
    }
    hr = SafeArrayUnaccessData(psaItems);
    if (FAILED(hr))
      throw _com_error(hr);
  }
  else
  {
    throw _com_error(hr);
  }

  hr = SafeArrayDestroy(psaItems);
  if (FAILED(hr))
    throw _com_error(hr);
}