Search code examples
c++c++14api-designc++17

Const-correct accessor to vector of pointers without transfer of ownership in abstract interface


I'm designing a library from scratch and want to get the public API as good as possible. I want the compiler to yell at me for misuse. Therefore, I've imposed the following rules on myself:

  1. true (i.e. deep and complete) const correctness throughout the whole library

    All things (local variables, member variables, member functions), which are not expected to change are declared const. That constness should propagate to all nested members and types.

  2. explicit and expressive ownership

    In line with the C++ Core Guidelines I define that as (iff in the mathematical sense of if and only if):

    1. function arguments are unique_ptr<T> or T&& iff the function is consuming it (i.e. taking ownership)
    2. function arguments are shared_ptr<const T> or T const& iff the function is only reading it
    3. function arguments are shared_ptr<T> or T& iff the function is modifying it without taking ownership
    4. return values are unique_ptr<T> or T iff the function transfers ownership to the caller
    5. return values are shared_ptr<const T> or T const& iff the caller should only read it (though the caller can construct a copy of it - given T is copyable)
    6. no functions should return shared_ptr<T>, T& or T* (as it would allow for uncontrollable side effects, which I try to avoid by design)
  3. hidden implementation details

    For now I'm going with abstract interfaces with factories returning the implementation as a unique_ptr<Interface>. Though, I'm open for alternative patterns which solve my problem described below.

I don't care about virtual table lookups and want to avoid dynamic casts by all means (I see them as a code smell).


Now, given two classes A and B, where B owns a variable number of As. As well we've the B-implementation BImpl (implementation of A is probably not of use here):

class A
{};

class B {
 public:
  virtual ~B() = default;
  virtual void addA(std::unique_ptr<A> aObj) = 0;
  virtual ??? aObjs() const = 0;
};

class BImpl : public B {
 public:
  virtual ~BImpl() = default;
  void addA(std::unique_ptr<A> aObj) override;
  ??? aObjs() const override;

 private:
  std::vector<unique_ptr<A>> aObjs_;
};

I'm stuck with the return value of Bs getter to the vector of As: aObjs().
It should provide the list of As as read-only values without transfer of ownership (rule 2.5. above with const correctness) and still provide the caller easy access to all As, e.g. for use in range-based for or standard algorithms such as std::find.

I came up with the following options for those ???:

  1. std::vector<std::shared_ptr<const A>> const&

    I would have to construct a new vector each time I call aObjs() (I could cache it in BImpl). That feels not only inefficient and needlessly complex, but seems also very suboptimal.

  2. Replace aObjs() by a pair of functions (aObjsBegin() and aObjsEnd()) forwarding the constant iterator of BImpl::aObjs_.

    Wait. I would need to make that unique_ptr<A>::const_iterator a unique_ptr<const A>::const_iterator to get my beloved const correctness. Again nasty casts or intermediate objects. And the user couldn't use it easily in range-based for.

What obvious solution am I missing?


Edit:

  • B should always be able to modify the As it is holding, thus declaring aObjs_ as vector<std::unique_ptr<const A>> is not an option.

  • Let B adhere to the iterator concept to iterate over the As, is neither an option as B will hold a list of Cs and a specific D (or none).


Solution

  • With range-v3, you may do

    template <typename T>
    using const_view_t = decltype(std::declval<const std::vector<std::unique_ptr<T>>&>() 
                            | ranges::view::transform(&std::unique_ptr<T>::get)
                            | ranges::view::indirect);
    
    class B
    {
    public:
        virtual ~B() = default;
        virtual void addA(std::unique_ptr<A> a) = 0;    
        virtual const_view_t<A> getAs() const = 0;
    };
    
    class D : public B
    {
    public:
        void addA(std::unique_ptr<A> a) override { v.emplace_back(std::move(a)); }
        const_view_t<A> getAs() const override {
            return v | ranges::view::transform(&std::unique_ptr<A>::get)
                     | ranges::view::indirect;
        }
    
    private:
        std::vector<std::unique_ptr<A>> v;
    };
    

    And then

    for (const A& a : d.getAs()) {
        std::cout << a.n << std::endl;   
    }
    

    Demo