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

slot_map: Do not require Container to provide reverse_iterator typedefs. #122

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Quuxplusone
Copy link
Contributor

Attn: @Masstronaut

With this patch, slot_map<T,K,C> will propagate the "reverse-iterability" of C<T>, both the existence of the member typedefs and the existence of the member functions rbegin, rend, crbegin, and crend (as a group, not individually).

Is this desirable? I tend to think yes, because implementing reverse-iterability for a custom container is usually wasted busywork. OTOH, it could alternatively be addressed by some library (either SG14 or Boost or otherwise) providing a reversible mix-in:

template<class Ctr>
struct reversible : public Ctr {
    using reverse_iterator = std::reverse_iterator<typename Ctr::iterator>;
    using const_reverse_iterator = std::reverse_iterator<typename Ctr::const_iterator>;
    reverse_iterator rbegin() { return reverse_iterator(this->end()); }
    reverse_iterator rend() { return reverse_iterator(this->begin()); }
    // ... and so on ...
};

template<class T> using reVector = reversible<TestContainer::Vector<T>>;
slot_map<int, std::pair<int,int>, reVector> sm;  // OK

BTW, the fact that this reVector alias template can be passed to a template template parameter is very cool. I was worried that it wasn't going to be that easy.

@mattreecebentley
Copy link
Contributor

Seems reasonable to me - does anyone have any objections to this? I'm not familiar enough with the code to comment as to side-effects.

The member function `sm.reserve_slots(n)` used to call `reserve` (and nothing
else) on the slots container. This is not useful functionality. After this
patch, `sm.reserve_slots(n)` resizes the slots container to `n`. It does not
touch the values container, since there is a valid use-case for "many slots,
relatively few values."

The member function `sm.reserve(n)` used to call `reserve` on all three
containers. After this patch, `sm.reserve(n)` still calls `reserve` on the
values container, and also on the reverse_map container, which is always
the same size as the values container by definition. It now *resizes*
the slots container to `n`, since there is no way for the programmer to
use the `n` capacity of the values container without taking up `n` slots
anyway.

N.B.: For container types that do not provide a suitable `reserve()` method,
the methods `sm.reserve(n)` and `sm.reserve_slots(n)` have exactly the same
effects.

The accessor `sm.capacity_slots()` has been renamed to `sm.slot_count()`
for clarity, and returns the `size()` of the slots container.

The accessor `sm.capacity()` still returns the `capacity()` of the
values container. It now SFINAEs away if the container does not provide
a suitable `capacity()` method.
`slot_map<T,K,C>` will now propagate the "reverse-iterability" of `C<T>`,
both the existence of the member typedefs and the existence of the
member functions `rbegin`, `rend`, `crbegin`, and `crend` (as a group,
not individually).
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.

2 participants