Skip to content

Conversation

@ssoelvsten
Copy link
Contributor

@ssoelvsten ssoelvsten commented Jul 27, 2022

Closes #196 .

Proposed Changes

  • Deprecates lambda in favour of map (with deprecation warnings)
  • Deprecates exclude_lambda in favour of a new filter_map (with deprecation warnings). This is such that a more consistent naming scheme is provided.
    • Currently a std::pair<T,bool> is used. Is it of interest to also support a std::optional<T> introduced in C++17? Based on this StackOverflow a pair is in terms of performance faster if the element constructor is cheap.
  • Fixes the documentation for exclude_lambda_t has the meaning of .first and .second the wrong way around compared to its implementation.

Possible Changes (but I cannot tell whether these are desired)

  • pipelining/map.h also has alternatives to map, i.e. map_temp, map_sink and push_map. Should the same also be made for pipelining/filter.h and pipelining/filter_map.h?
  • tpie/pipelining.h imports pipelining/map.h but not pipelining/filter.h. Is this on purpose?
  • There are no unit tests for map, filter and filter_map and looking at test/pipelining.cpp and test/pipelining2.cpp I cannot exactly tell how I should add these.

@ssoelvsten ssoelvsten force-pushed the issue_196 branch 3 times, most recently from b2dbca2 to c8a2e8b Compare July 27, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pipelining duplication: lambda_t and map_t

2 participants