1

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?

Andrew Truckle
  • 17,769
  • 16
  • 66
  • 164

2 Answers2

3

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);
}
Vlad Feinstein
  • 10,960
  • 1
  • 12
  • 27
  • Thanks for this answer. I have tried it out and from my tests it seems to work well! I will now ask a related question to see if this template can be further adapted to cater for one of my other convert methods. – Andrew Truckle Jan 05 '21 at 08:55
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.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111