Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++/WinRT] Helpers to make it easier to copy data from one collection type (std::initializer_list, etc.) to another (IVector, ValueSet, etc.) #356

Open
JaiganeshKumaran opened this issue Sep 6, 2023 · 4 comments
Labels
feature-request New feature or request

Comments

@JaiganeshKumaran
Copy link

JaiganeshKumaran commented Sep 6, 2023

There are a few classes, such as ValueSet, in the inbox WinRT API set that implement collection interfaces. Currently populating them requires multiple IVector::Append or IMap::Insert calls, which is inconvenient. Consider adding a std::initializer_list constructor (and maybe others such as taking iterators) for simplified construction.

@kennykerr
Copy link
Contributor

I've tagged a few project maintainers. If one of them is available to offer guidance and mentorship for this issue, they will reach out according to the contributing guide to discuss and agree on an approach.

@microsoft/cppwinrt-maintainers

For what it's worth, the set of available constructors is defined by the class itself in metadata. It does not seem like something that we can or should infer beyond what's in metadata.

@oldnewthing
Copy link
Member

While we often add convenience methods, those methods typically map 1:1 to an existing API. Examples:

  • IMap<K, V>::TryLookup() is IMap<K, V>::Lookup() with a different error reporting mechanism.
  • IBuffer::data() is IBufferByteAccess::Buffer().
  • Iterator ++ and * operators are MoveNext() and Current, respectively.

There is no way to construct a ValueSet from an existing iterable of key-value pairs, nor is there a way to bulk-insert items into a ValueSet or IMap, so it would be misleading to offer these as extension methods (which imply a direct mapping to the API, and in particular imply atomicity).

If you feel so inclined, you could add a winrt_map_inserter to WIL.

template<typename K, typename V>
struct winrt_map_inserter : public std::iterator< std::output_iterator_tag, void, void, void, void >
{
    winrt_map_inserter(IMap<K, V> const& map) : map(map) {}

    template<typename U>
    winrt_map_inserter& operator=(U const& kvp)
    {
        map.Insert(get<0>(kvp), get<1>(kvp));
        return *this;
    }

    winrt_map_inserter& operator*() { return *this; }
    winrt_map_inserter& operator++() { return *this; }
    winrt_map_inserter& operator++(int) { return *this; }

    IMap<K, V> map;
};

Then you could write

std::copy(list.begin(), list.end(), wil::winrt_map_inserter(valueSet));

to bulk-fill a ValueSet or any other map type.

Note that IKeyValuePair<K, V> already supports get<N> via ADL, so this code is generic to IKeyValuePair<X, Y> or std::pair<X, Y> or std::tuple<X, Y> as long as X is convertible to K and Y is convertible to V.

@jonwis
Copy link
Member

jonwis commented Sep 6, 2023

I'm currently working on a version of a "make value set" helper that takes an init-list of std::wstring_view and std::variant<many> ... we have lots of code that builds ValueSet types and the resulting binary impact is very high. This inserter would be fine too. I'll share a gist of it eventually.

+1 on the "this should go wil" though.

@kennykerr kennykerr transferred this issue from microsoft/cppwinrt Sep 6, 2023
@dunhor dunhor added the feature-request New feature or request label Sep 19, 2023
@dunhor dunhor changed the title Inject initializer_list constructor for classes implementing collection interface [C++/WinRT] Helpers to make it easier to copy data from one collection type (std::initializer_list, etc.) to another (IVector, ValueSet, etc.) Sep 19, 2023
@JaiganeshKumaran
Copy link
Author

For vectors, at least you could add an AppendRange method (similar to C++23's std::vector) that combines GetMany and ReplaceAll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants