Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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).

like image 747
Torbjörn Avatar asked Aug 31 '16 19:08

Torbjörn


3 Answers

Instead of trying to return the vector directly, you can return a wrapper of a vector that allows you to access the content only with const pointers. It might sound complicated, but it's not. Just make a thin wrapper and add a begin() and end() function to allow iteration:

struct BImpl : B {
    virtual ~BImpl() = default;
    void addA(std::unique_ptr<A> aObj) override;

    ConstPtrVector<A> aObjs() const override {
        return aObjs_;
    }

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

ConstPtrVector will look like this:

template<typename T>
ConstPtrVector {
    ConstPtrVector(const std::vector<T>& vec_) : vec{vec_} {}

    MyConstIterator<T> begin() const {
        return vec.begin();
    }

    MyConstIterator<T> end() const {
        return vec.end();
    }

private:
    const std::vector<T>& vec;
};

And You can Implement MyConstIterator in a way that will return pointers as const:

template<typename T>
struct MyConstIterator {
    MyConstIterator(std::vector<unique_ptr<T>>::const_iterator it_) : it{std::move(it_)} {}

    bool operator==(const MyConstIterator& other) const {
        return other.it == it;
    }

    bool operator!=(const MyConstIterator& other) const {
        return other.it != it;
    } 

    const T* operator*() const {
        return it->get();
    }

    const T* operator->() const {
        return it->get();
    }

    MyConstIterator& operator++() {
        ++it;
        return *this;
    }

    MyConstIterator& operator--() {
        --it;
        return *this;
    }

private:
    std::vector<unique_ptr<T>>::const_iterator it;
};

Of course, you can generalize this iterator and wrapper by implementing a vector like interface.

Then, to use it, you can use range based for loop or a classic iterator based loop.

BTW: There is nothing wrong with non owning raw pointers. As long as they are still non owning. If you want to avoid error due to raw pointers, look at observer_ptr<T>, it might be useful.

like image 177
Guillaume Racicot Avatar answered Oct 16 '22 00:10

Guillaume Racicot


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

like image 3
Jarod42 Avatar answered Oct 16 '22 01:10

Jarod42


template<class It>
struct range_view_t {
  It b{};
  It e{};
  range_view_t(It s, It f):b(std::move(s)), e(std::move(f)) {}
  range_view_t()=default;
  range_view_t(range_view_t&&)=default;
  range_view_t(range_view_t const&)=default;
  range_view_t& operator=(range_view_t&&)=default;
  range_view_t& operator=(range_view_t const&)=default;

  It begin() const { return b; }
  It end() const { return e; }
};

here we start with a range of iterators.

We can make it richer with range_view_t remove_front(std::size_t n = 1)const, bool empty() const, front() etc.

We can augment it using the usual techniques, conditionally adding operator[] and size if It has a random_access_iterator_tag category, and making remove_front silently bound n.

Then going one step further, we write array_view_t:

template<class T>
struct array_view_t:range_view<T*> {
  using range_view<T*>::range_view;
  array_view_t()=default; // etc
  array_view_t( T* start, std::size_t length ):array_view_t(start, start+length) {}
  template<class C,
    std::enable_if_t
      std::is_same< std::remove_pointer_t<data_type<C>>, T>{}
      || std::is_same< const std::remove_pointer_t<data_type<C>>, T>{},
      , int
    > =0
  >
  array_view_t( C& c ):array_view_t(c.data(), c.size()) {}
  template<std::size_t N>
  array_view_t( T(&arr)[N] ):array_view_t( arr, N ) {}
};

which abstracts looking at the contents of a contiguous container.

Now your BImpl returns an array_view_t< const std::unique_ptr<A> >.

This level of abstraction is basically free.


If that isn't enough, you type erase random access iteration of T, then return a range_view_t< any_random_access_iterator<T> >, where in this case T is const std::unique_ptr<A>.

We could also erase the ownership semantics, and just be a range_view_t< any_random_access_iterator<A*> > after throwing in a range adapter.

This level of type erasure is not free.


For utter insanity, you can stop using smart pointers or interfaces.

Describe your interfaces using type erasure. Pass out any's wrapped with the type erasure. Nearly everything uses value semantics. If you consume a copy, take by value, then move out of that value. Avoid persistent references to objects. Short term references are references, or pointers if they are optional. These are not stored.

Use names instead of addresses, and use a registry somewhere to get at the items, when you cannot afford to use values.

like image 3
Yakk - Adam Nevraumont Avatar answered Oct 16 '22 02:10

Yakk - Adam Nevraumont