2

I have the following problem and I wonder whether there's a better way to solve it:

class myObj {
public:
    typedef std::shared_ptr<myObj> handle;
    typedef std::shared_ptr<const myObj> const_handle;
    int someMethod() { ... }
    int someConstMethod() const { ... }
};

Now what I need is a container class that somehow allows you to modify or read a collection of myObj depending on its own constness, like so:

class myCollection {
public:
    typedef std::list<myObj::handle> objList;
    typedef std::list<myObj::const_handle> const_objList;

    inline objList& modify() { return _obl; }

    // it would be nice to do this, but it won't compile as 
    // objList and const_objList are completely different types
    inline const_objList& read() const { return _obl; } // doh! compile error...

    // returning a const objList won't help either as it would return non-const
    // handles, obviously.

    // so I am forced to do this, which sucks as i have to create a new list and copy
    void read(const_objList &l) {
        std::for_each(
            _obl.begin(), 
            _obl.end(), 
            [&l] (myObj::handle &h) { l.push_back(h); } 
            // ok as handle can be cast to const_handle
        ); // for_each
    }

private:
    objList _obl;
};

So this solution actually works as a const myCollection would only allow you to get a list of const_handle which only allows you to call non-modifying methods of myObj (GOOD).

The problem is that the "read" method is really ugly (BAD).

Another method would be to expose somehow the list methods and return const_handle and handle as needed but it's a lot of overhead, especially if you want to use something more complex than a list.

Any idea?

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Odds are `std::list` is not the correct container, because it very rarely is the right container. The situations where `std::list` is a better idea than `std::vector` are relatively rare, and even more rare are the situations where it is better than both `std::vector` and `std::deque`. – Yakk - Adam Nevraumont Jul 05 '13 at 03:17
  • How about getters/setters for the list, i.e. providing an interface to change the list rather than returning it? Iterators are another way if you only need to change the elements (w/o removing or inserting). – dyp Jul 05 '13 at 04:03

2 Answers2

0

Given what you stated that you want to accomplish, I don't think that your solution is too bad. Imagine that some other code may be modifying the internal collection, like adding or removing values. Returning a copy of the current state of the collection is safe for client code, since it can work on the copy, without the danger of element being deleted in the meantime. But I digress, this is getting into threading issues and may not be relevant.

You could use prettier:

  inline const_objList read()  const
   { 
       const_objList cl(_obl.begin(), _obl.end());
       return cl; 
   } 

However, I do think that your problems derive from mixing two types of constness: constness of the members of the collection versus the constness of the collection itself.

Instead of Modify and Read methods, that deal with the list as a whole, I would try exposing const and non-const iterators to internal list, through corresponding const and non-const methods returning said iterators.

But this immediately begs the question: why then have myCollection in the first place?

Creating entirely new collection type around std::list doesn't seem needed, unless you get a lot of proverbial bang for the buck from other, added functionality that is not visible in your sample.

You can then make your added functionality free methods that take std::list of your handles as the input. Not everything requires an object and operations on objects need not necessarily be member methods, unless access to private data is required.

You mentioned maybe using another container instead of the list. But your class, as is, won't do it, unless you have a template, where template parameter can be one of STL containers. Which then implies that you should expose iterators. Namely, if you foresee changing the internal collection type, you would want to make the public interface to myCollection transparent regarding the collection type. Otherwise, clients will have to recompile each time you change your mind about the internal implementation.

EDIT -----

Finally, if implementing iterators (while interesting and most correct) is too much, why not go for simple getters like in this SO post:

smart pointer const correctness

I'll quote the topmost answer by Rüdiger Stevens (it assumes vector instead of list):

template <typename T>
class MyExample
{
  private:
    vector<shared_ptr<T> > data;

  public:
    shared_ptr<const T> get(int idx) const
    {
      return data[idx];
    }
    shared_ptr<T> get(int idx)
    {
      return data[idx];
    }
    void add(shared_ptr<T> value)
    {
      data.push_back(value);
    }
};
Community
  • 1
  • 1
Tony
  • 1,566
  • 2
  • 15
  • 27
0

A list-of-pointers-to-T is not a list-of-pointers-to-constant-T.

std::list<std::shared_ptr<int>> a;
std::list<std::shared_ptr<const int>>& ra = a; // illegal but imagine it's not
std::shared_ptr<const int> x = std::make_shared<const int>(42);
ra.push_back(x); // totally legal, right?
++**a.begin(); // oops... just incremented a const int

Now a list-of-pointers-to-T is, conceptually, a constant-list-of-constant-pointers-to-constant-T, but std::list<std::shared_ptr<T>> does not support such a deep const propagation. const std::list<std::shared_ptr<T>> contains constant pointers to non-constant objects.

You can write your own variant of list<> or your own variant of shared_ptr<> that have such support. It probably won't be very easy though. A const_propagating_shared_ptr is probably the easier of the two. It would have to encapsulate an std::shared_ptr<T> object and forward almost everything to it as-is. As opposed to std::shared_ptr<T> it would have separate const and non-const versions of operator->, operator*() and get().

n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243