YarikTH / ureact

Functional reactive programming library for c++
Boost Software License 1.0
155 stars 10 forks source link

Improve `ureact::collect` #111

Closed YarikTH closed 1 year ago

YarikTH commented 1 year ago

Description

Paper about std::ranges::views::enumerate mentioned the following code:

std::vector<double> v;
enumerate(v) | to<std::vector>(); // std::vector<std::tuple<std::size_t, double>>
enumerate(v) | to<std::map>(); // std::map<std::size_t, double>

Currently ureact::collect supports only std::vector like containers that can be specialized with single type. It would be nice to have the possibility to collect pair-like values in associative containers like std::map.

It worth to check is it supported in existing ranges::to<> implementations.

Anyway, ureact::collect is dangerous and mostly meant to be used in testing code, so this feature is not important.

YarikTH commented 1 year ago

Test code have written to check the feature

TEST_CASE( "CollectAssociative" )
{
    ureact::context ctx;

    auto src = ureact::make_source<std::pair<std::string, int>>( ctx );
    auto src_tuple = ureact::transform(
        src, []( const auto& pair ) { return std::make_tuple( pair.first, pair.second ); } );

    ureact::signal<std::map<std::string, int>> collected_map;
    ureact::signal<std::map<std::string, int>> collected_map_2;

    SUBCASE( "Functional syntax" )
    {
        collected_map = ureact::collect<std::map>( src );
        collected_map_2 = ureact::collect<std::map>( src_tuple );
    }
    SUBCASE( "Piped syntax" )
    {
        collected_map = src | ureact::collect<std::map>;
        collected_map_2 = src_tuple | ureact::collect<std::map>;
    }

    src << std::pair( "A", 1 );
    src << std::pair( "B", 2 );
    src << std::pair( "C", 3 );

    using namespace std::string_literals;
    const auto expected = std::map{
        std::pair( "A"s, 1 ),
        std::pair( "B"s, 2 ),
        std::pair( "C"s, 3 ),
    };

    CHECK_EQ( collected_map.get(), expected );
    CHECK_EQ( collected_map_2.get(), expected );
}
YarikTH commented 1 year ago

std::to proposal https://open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1206r7.pdf

auto l = std::views::iota(1, 10);

// create a vector with the elements of l
auto vec = ranges::to<std::vector<int>>(l); // or vector{std::from_range, l};

//Specify an allocator
auto b = ranges::to<std::vector<int, Alloc>>(l, alloc); // or vector{std::from_range, l, alloc};

//deducing value_type
auto c = ranges::to<std::vector>(l);

// explicit conversion int -> long
auto d = ranges::to<std::vector<long>>(l);

//Supports converting associative container to sequence containers
auto f = ranges::to<std::vector>(m);

//Supports converting sequence containers to associative ones
auto g = ranges::to<std::map>(f);

//Pipe syntaxe
auto g = l | ranges::view::take(42) | ranges::to<std::vector>();

//Pipe syntax with allocator
auto h = l | ranges::view::take(42) | ranges::to<std::vector>(alloc);

//The pipe syntax also support specifying the type and conversions
auto i = l | ranges::view::take(42) | ranges::to<std::vector<long>>();

// Nested ranges
std::list<std::forward_list<int>> lst = {{0, 1, 2, 3}, {4, 5, 6, 7}};
auto vec1 = ranges::to<std::vector<std::vector<int>>>(lst);
auto vec2 = ranges::to<std::vector<std::deque<double>>>(lst);
YarikTH commented 1 year ago

Some checks in the godbolt https://godbolt.org/z/jqePW8a7h. It seems that it is impossible to accept both concrete type and parameterized type at the same time if it is not an overloaded template function.

Optional passing of allocator is another can of worm I don't want to touch for now. The same with nested containers. It doesn't look practical for ureact anyway.

YarikTH commented 1 year ago

Update on godbolt https://godbolt.org/z/9hsvjfzfv.

YarikTH commented 1 year ago

Associative container support is added. But there is at least some ambiguity with std::map: it has both .insert() method that doesn't override previous values and operator[] that does override. It is not clear which behavior is desired. For now operator[] has bigger priority than .insert().

YarikTH commented 1 year ago

So, no allocator support for now and no passing of specific class for now (it require a new adaptor name like collect_in<std::vector>). I have no idea how to use allocators, so it's hard to make by me.

YarikTH commented 1 year ago

Closing for now. Reopen if further improvements are wanted