-1

In C++, I have a class that contains two members which are sorted containers. The containers store the same object type, but one is sorted in ascending order and the other in descending order. Note: the object type in each container is the same, but the actual object instances stored in the two containers are different. In other words, the data is not duplicated.

I have several template methods which visit items in a container, with the template argument saying which container to visit (i.e. the one sorted in ascending order, or the other).

Regardless of which container I iterate on, I perform the exact same operations. But the order in which I visit the items is important.

So I'm looking for an abstraction or language construct that allows me to work with the two containers in a generic way.

Here is an example to help explain what I'm trying to do:

#include <set>
#include <iostream>

class MySets
{
    public:
        using fwd_set_t = std::set<int, std::less<int>>;
        using rev_set_t = std::set<int, std::greater<int>>;

        // BEGIN: added in edit
        using MyIterator = fwd_set_t::iterator;
        using MyConstIterator = fwd_set_t::const_iterator;

        // is this safe??? the compiler isn't complaining that I'm using
        // fwd_set_t::iterator and rev_set_t::iterator interchangeably
        template <bool IS_FWD> inline MyConstIterator begin() const
        {
            return (IS_FWD ? fwd_set_.begin() : rev_set_.begin());
        }

        template <bool IS_FWD> inline MyConstIterator end() const
        {
            return (IS_FWD ? fwd_set_.end() : rev_set_.end());
        }

        template <bool IS_FWD> inline void printSetAlternate() const
        {
            for (auto iter = begin<IS_FWD>(); iter != end<IS_FWD>(); ++iter)
            {
                std::cout << "  " << (*iter) << "\n";
            }
        }
        // END: added in edit


        MySets()
            : fwd_set_({10, 20, 30, 40, 50})
            , rev_set_({11, 21, 33, 44, 55})
            {
            }

        template <bool IS_FWD> inline void printSet() const
        {
            //auto const& s = (IS_FWD ? fwd_set_ : rev_set_); // ERROR - different types
            //for (auto const& n : s) { std::cout << "  " << n << "\n"; }

            if (IS_FWD) { for (auto const& n : fwd_set_) { std::cout << "  " << n << "\n"; } }
            else        { for (auto const& n : rev_set_) { std::cout << "  " << n << "\n"; } }
        }

    private:
        fwd_set_t fwd_set_;
        rev_set_t rev_set_;
};

int main(int argc, char* argv[])
{
    MySets ms;

    std::cout << "fwd:\n";
    ms.printSet<true>();

    std::cout << "rev:\n";
    ms.printSet<false>();

    return 0;
}

The key part of the example is the printSet() method. The commented-out code gives a compiler error ("operands to ‘?:’ have different types"), but shows the intent of what I'd like to do. Imagine this function wasn't trivial, but instead had a fair amount of logic to perform on each item in the set. I'd rather not copy and paste the code as I did in the example.

So I feel like there should be some kind of abstraction or language feature I can use to achieve the intended result, but I'm not sure what that is.

Note: I am forced to use C++14, so any solution that uses newer language features won't work for me.

Edit: I added some code to the example above. The compiler does not complain about using a fwd_set_t::iterator to also refer to a rev_set_t::iterator. And calling printSetAlternate() works as expected. My question is, is this safe to do? I would like to combine this with the suggestion from @NathanOliver below. The actual use-case involves passing iterators around to different functions.

Matt
  • 952
  • 2
  • 8
  • 17
  • Why not making templated sort functions for all kinds of containers (as done in the c++ standard library with [`std::sort()`](https://en.cppreference.com/w/cpp/algorithm/sort))? – πάντα ῥεῖ Oct 02 '20 at 19:28
  • 1
    `using set_t = std::set>;` so they're the same type, and then in the constructor `: fwd_set_({10, 20, 30, 40, 50}, std::less{}), rev_set_({11, 21, 33, 44, 55}, std::greater{})`. This will make the code _slightly_ slightly slower, but also they can share code, resulting in smaller bytecode, which may offset the performance. – Mooing Duck Oct 02 '20 at 19:55
  • 1
    Another option is keep the container sorted one direction, and then use `reverse_iterator`. The standard containers with bidirectional or better iterators all have `rbegin()`/`rend()` methods to make this easier – Dave S Oct 02 '20 at 20:02
  • @DaveS you should spell that out in an answer, since it's the correct answer and there are different answers already getting upvoted. – parktomatomi Oct 02 '20 at 20:21

2 Answers2

2

You can write a couple private helper functions and tags like

struct fwd_tag_t{};
struct rev_tag_t{};
auto& getSet(fwd_tag_t) const { return fwd_set_; }
auto& getSet(rev_tag_t) const { return rev_set_; }

and then use tag dispatch to call the one to get the correct set like

template <bool IS_FWD> inline void printSet() const
{
    auto const& s = getSet(std::conditional_t<IS_FWD, fwd_tag_t, rev_tag_t>{});
    for (auto const& n : s) { std::cout << n << " "; }
    std::cout << "\n";
}

If you can upgrade to C++17, you could removes the tags and the overloads and change getSet to just be

template <bool IS_FWD> inline auto& getSet() const
{
    if constexpr(IS_FWD) 
        return fwd_set;
    else 
        return rev_set;
}

and then printSet would become

template <bool IS_FWD> inline void printSet() const
{
    auto const& s = getSet();
    for (auto const& n : s) { std::cout << n << " "; }
    std::cout << "\n";
}
NathanOliver
  • 171,901
  • 28
  • 288
  • 402
  • I think this gets me most of the way there. I actually had a construct like this, but I missed the tag dispatch part (or explicit specialization, as someone else suggested). Without tag dispatch/explicit specialization, I get the compiler error "inconsistent deduction for auto return type". – Matt Oct 02 '20 at 20:31
1

Another option, depending on what you're trying to do with it is to keep the data once, and use reverse_iterator.

void printSet(bool is_fwd)
{
    auto print = [](auto first, auto last) {
       while(first != last) {  std::cout << *first++ << "\n"; }
    };
    if (is_fwd) {
      print(fwd_set_.begin(), fwd_set.end());
    }  else {
      print(fwd_set.rbegin(), fwd_set.rend());
    }
}

Feel free to make printSet a template, and use if constexpr or tag dispatching. You can also make the lambda a separate member function

Dave S
  • 20,507
  • 3
  • 48
  • 68
  • My concern with this is the potential impact on performance in terms of data locality/caching/pre-fetching. IOW, I'm assuming I get some pre-fetch benefit on the forward iterated set, since the CPU is likely to grab not only begin() but several subsequent elements as well. I didn't mention this in the original post, but I'm actually using a boost::flat_set, which keeps data in contiguous memory (like a vector). My real goal here is to create a very generic structure so that I can easily test performance with different containers. – Matt Oct 02 '20 at 20:41
  • @Matt Surely, using a duplicate data structure has a negative effect on cache performance? Besides, I'm pretty sure newer processors do backwards prefetch. – parktomatomi Oct 05 '20 at 03:34
  • The data isn't duplicated. I.e., fwd_set and rev_set have different data. The same object type, but actual object instances are different. (I will edit my question to make that more clear.) – Matt Oct 05 '20 at 14:43