STL: Initializing a container with an unconstructed stateful comparator

Question

This has been running through my mind as a possible solution to an issue, however as it is a fairly obvious technical violation of something in C++, I wanted to know how likely to it is to fail, whether there is another fairly obvious approach, etc. I'm hoping this doesn't get into a flamewar about undefined behavior, but considering the topic I do expect a little bit.

This is not the code I'm writing, I'm hoping it's not too simplified to not describe what I am attempting to do.

class Code
{
public:
  bool read(short slot, short& val);
  bool read(short slot, long& val);
  bool read(short slot, double& val);
  // etc
protected:
  unsigned char* m_data;
};
typedef boost::shared_ptr<Code> CodePtr;

class SortedBase
{
protected:
   class Sorter : public std::binary_function<CodePtr,CodePtr,bool>
   {
   protected:
     inline Sorter() {}
     virtual ~Sorter() {}
   public:
     virtual bool operator()(CodePtr left, CodePtr right) PURE;
   };

   inline SortedBase(Sorter* s):m_codeList(s) {}

   typedef std::set<CodePtr,Sorter> TSortedCode;
   TSortedCode m_codeList;
public:
   virtual ~SortedBase() {}
   void fetch(); // populates m_codeList
};

template<class SORT1, class SORT2, class SORT3, class SORT4, class SORT5>
class SortedObject5 : public SortedBase
{
public:
  SortedObject5():SortedBase(m_sorter),m_sorter(this) {}

  something_interesting find(SORT1 val1, SORT2 val2, SORT3 val3, SORT4 val4, SORT5 val5);
protected:
  typedef SortedObject5<SORT1,SORT2,SORT3,SORT4,SORT5> my_class;
  class MySorter : public Sorter
  {
  public:
    MySorter(const my_class& parent):m_parent(parent) {}
    virtual operator()(CodePtr left, CodePtr right);
  protected:
    const my_class& m_parent;
  }

  MySorter m_sorter;
};

The intent here

I've often found when writing template classes that having a non-template base class with as much of the factored logic as possible is useful to both have some common class other code can reference and reduce the amount of code duplication, especially when making five different versions of the same class with different numbers of template parameters.

In this case the CodePtr is generated elsewhere in the code (although I did write it) and I would like to find elements based on an arbitrary number of arbitrary datatypes. I considered a std::multimap at first but the key would end up being a wrapper to (or a copy of a significant chunk of) the CodePtr again.

The problem

I am passing the stateful sorter functor SortedObject5<>::my_sorter to the constructor of SortedBase::m_codeList. However because the stateful sorter being in a sublcass, is fairly obviously not constructed at the point that the STL set is constructed.

I'm wondering if this is an issue if I don't make any inserts or searches in m_codeList from either constructor.

Stateful sorter disclaimer

I formally ASSERT() that the rules used by any stateful sort functor will change only while either the STL containers it controls are empty or will be clear()ed shortly afterwards.

Answer 1

The std::set<CodePtr,Sorter> object stores an instance of Sorter by value so when you construct it with a Sorter* (did you mean that to be a reference not a pointer?) it will slice the object and only keep the base part.

That means the Sorter copy constructor will run and make a copy of an uninitialized object. Undefined behaviour ensues.

That's assuming you can even create an instance of Sorter, if it's an abstract type you won't be able to (I don't know what your PURE does but I assume you're making the function pure virtual)

@Angew's comment suggest a good approach, the base from member idiom will allow you to ensure the m_sorter object is initialized first, which is part of the problem. That doesn't help the issue of slicing though, to solve that you'd need some wrapper around the sorter e.g.

typedef std::function<bool(const CodePtr&,const CodePtr&)> SorterFunc;
typedef std::set<CodePtr, SorterFunc> TSortedCode;

And then pass the wrapper to the set constructor:

inline SortedBase(SorterFunc s) : m_codeList(s) {}

If you construct the std::function from the derived type it won't be sliced. It will be copied though, but you can prevent that by using a reference wrapper:

  SortedObject5() : BaseFrommember(this), SortedBase(SorterFunc(std::ref(m_sorter))) { }

Where m_sorter is already initialized, because it is stored in the BaseFromMember base class, using the base-from-member idiom.

This:

1. creates the m_sorter first so you don't do anything with an uninitialized object
2. passes it by reference to a SorterFunc object
3. uses a copy of that SorterFunc (still holding a reference to m_sorter) as the comparision function for the std::set

If you don't want to use the base-from-member idiom then it's still easy to avoid the undefined behaviour of your original code, just default construct the set (instead of passing it an uninitialized object) then assign a new value to it before you start populating it:

SortedObject5() : m_sorter(this)
{
  this->m_codeList = TSortedCode(SorterFunc(boost::ref(m_sorter)));
}

No new base classes, no extra templates, no undefined behaviour.

Here's the working code in full:

class SortedBase
{
protected:
   class Sorter : public std::binary_function<CodePtr,CodePtr,bool>
   {
   protected:
     Sorter() {}
     virtual ~Sorter() {}
   public:
     virtual bool operator()(const CodePtr& left, const CodePtr& right) = 0;
   };

   typedef boost::function<bool(const CodePtr&, const CodePtr&)> SorterFunc;

   typedef std::set<CodePtr,SorterFunc> TSortedCode;

   TSortedCode m_codeList;

public:
   virtual ~SortedBase() {}
   void fetch(); // populates m_codeList
};

template<class SORT1, class SORT2, class SORT3, class SORT4, class SORT5>
class SortedObject5 : public SortedBase
{
public:
  SortedObject5() : m_sorter(*this)
  {
    this->m_codeList = TSortedCode(SorterFunc(boost::ref(m_sorter)));
  }

protected:
  typedef SortedObject5<SORT1,SORT2,SORT3,SORT4,SORT5> my_class;

  class MySorter : public Sorter
  {
  public:
    MySorter(const my_class& parent):m_parent(parent) {}
    virtual bool operator()(const CodePtr& left, const CodePtr& right);
  protected:
    const my_class& m_parent;
  };

  MySorter m_sorter;
};