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

Add Python-like set operations to flat_set #2557

Merged
merged 5 commits into from
Nov 17, 2023

Conversation

AntoinePrv
Copy link
Member

  • Add flat-set doctest printer
  • Add Python-like set operations to flat_set

@AntoinePrv AntoinePrv requested review from JohanMabille and Klaim May 31, 2023 15:41
@AntoinePrv AntoinePrv self-assigned this May 31, 2023
@AntoinePrv AntoinePrv marked this pull request as draft June 1, 2023 14:11
@Klaim
Copy link
Member

Klaim commented Jun 9, 2023

I feel like it's not a good idea to wrap generic algorithms into member functions for convenience because

I'm not completely against adding these functions but could you make them free instead of member at least?

@AntoinePrv
Copy link
Member Author

The algorithms exists as member functions because they can leverage the specific internals of the class for better algorithmic complexity. Just like we don't use std::find on a map.

IMHO C++ <algorithm> header is confusing. There are "range manipulation" algorithms (filter, transform...) that are in fact generic and the C++20 range library does amazingly well. Then there are more complex algorithms (e.g. non-linear), and these cannot be decoupled from data structure. I think set operations fall in the second category.

@@ -30,6 +30,7 @@ namespace mamba::util
public:

using Base = std::vector<Key, Allocator>;
using Self = flat_set<Key, Compare, Allocator>;
Copy link
Member

@JohanMabille JohanMabille Nov 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking: I think we can simply use flat_set in the declaration and implementation, the template parameters will be deduced.

@AntoinePrv AntoinePrv marked this pull request as ready for review November 17, 2023 09:47
template <class InputIt1, class InputIt2, class Compare>
auto
set_disjoint(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2, Compare comp)
-> bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥲

@AntoinePrv AntoinePrv merged commit 2c8ec3c into mamba-org:main Nov 17, 2023
27 checks passed
@AntoinePrv AntoinePrv deleted the set-operations branch November 17, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants