4

I have a class, which supports cloning (via method clone). I have a bunch of its instances in a vector of std::unique_ptr.

Now, I want to create an std::set of same smart pointers from the above vector, ideally during its construction. The obvious design is as following:

#include <memory>
#include <set>
#include <vector>

class A
{
public:
    /// type of itself
    typedef A self;
    A() = default;
    A(const self& another) = default;
    virtual ~A() = default;

    std::unique_ptr<A> clone() const
    {
        return std::make_unique<A>();
    }
};

class SetOfA
{
public:
    SetOfA() = default;

    // Here is the method I would like to improve
    SetOfA(const std::vector<std::unique_ptr<A> >& data)
    {
        //do not like this loop, prefer this to be in initialization part?
        for (const auto& d : data) {
            set_of_a.insert(std::move(d->clone()));
        }
    }

private:
    std::set<std::unique_ptr <A> > set_of_a;
};

But is there a way to avoid this for loop in constructor and move the std::set construction into initialization part?

Evg
  • 25,259
  • 5
  • 41
  • 83
one_two_three
  • 501
  • 2
  • 10
  • 2
    Note that `std::move()` is redundant there. `clone()` returns a prvalue. – Evg Sep 29 '21 at 12:49
  • I believe you're out of luck, unless you count "write a separate cloning function for vectors" as a solution. – molbdnilo Sep 29 '21 at 12:57
  • 1
    C++20 `std::views::transform` comes close, but then `std::set` doesn't have a constructor taking a view. – aschepler Sep 29 '21 at 13:01
  • @aschepler you just need a intermediate function that calls the iterator constructor. `template Container construct(Range&& r) { return { r.begin(), r.end() }; }` – Caleth Sep 29 '21 at 14:23

2 Answers2

8

If you can use Boost, here is a possible solution:

SetOfA(const std::vector<std::unique_ptr<A>>& data) :
    set_of_a(
        boost::make_transform_iterator(data.begin(), std::mem_fn(&A::clone)),
        boost::make_transform_iterator(data.end(),   std::mem_fn(&A::clone)))
    { }
Evg
  • 25,259
  • 5
  • 41
  • 83
  • @JeJo Move iterators would leave the vector elements empty. Given the discussion of `clone` and the `const` vector in the question, I don't think that's wanted here. – aschepler Sep 29 '21 at 15:20
3

You could use std::transform to avoid for loop to insert values in a std::set.

SetOfA(const std::vector<std::unique_ptr<A> >&data) 
{
   std::transform(data.begin(), data.end(), std::inserter(set_of_a, set_of_a.begin()), [](auto& a){return a->clone();});
 // or better
 //std::transform(data.begin(), data.end(), std::inserter(set_of_a, set_of_a.begin()), std::mem_fn(&A::clone));
}

It compiles with C++14 and above. Please find tested version of it here https://godbolt.org/z/jzvaKovn7.

And to put the whole stuff in initialization part, you could use a lambda or a Functor, to convert data into set as shown below:

class SetOfA
{
    struct VectorToSetConverter
    {
        std::set<std::unique_ptr<A>> operator()  (const std::vector<std::unique_ptr<A> >&data) const
        {
            std::set<std::unique_ptr<A>> set_of_a;
            std::transform(data.begin(), data.end(), std::inserter(set_of_a, set_of_a.begin()), std::mem_fn(&A::clone));
            return set_of_a;
        }
   };

  public:
  
    SetOfA() = default;
    SetOfA(const std::vector<std::unique_ptr<A> >&data)
    : set_of_a(std::move(VectorToSetConverter()(data))) // move might not be necessary
    {
    }
    
  private:

    std::set<std::unique_ptr <A> > set_of_a;
};

https://godbolt.org/z/6aGPKndb6

Sumit Jha
  • 1,601
  • 11
  • 18