3

I have template class ItemContainer that actually is facade for a whole family of containers with different capabilities like sorting, indexing, grouping etc.

Implementation details are hidden in cpp. file using pimpl idiom and explicit instantiation. Template is instantiated only with well-known limited set of implementation classes that define the actual behavior of container.

Main template implements common functions supported by all containers - IsEmpty(), GetCount(), Clear() etc.

Each specific container specializes some functions that are supported only by it, e.g. Sort() for sorted container, operator[Key&] for key indexed container etc.

The reason for such design is that class is replacement for several legacy hand-made bicycle containers written by some prehistorics in early 90th. Idea is to replace old rotting implemenation with modern STL&Boost containers keeping old interface untouched as much as possible.

The problem

Such design leads to unpleasant situation when user tries to call unsupported function from some specialization. It compiles OK, but produces error on linking stage (symbol not defined). Not very user friendly behavior.

Example:

 SortedItemContainer sc; sc.IsEmpty(); // OK sc.Sort(); // OK IndexedItemContainer ic; ic.IsEmpty(); // OK ic.Sort(); // Compiles OK, but linking fails 

Of course, it could be completely avoided by using inheritance instead of specialization but I don't like to produce a lot of classes with 1-3 functions. Would like to keep original design.

Is there possibility to turn it into compile stage error instead of link stage one? I have a feeling that static assert could be used somehow.

Target compiler for this code is VS2008, so practical solution must be C++03 compatible and could use MS specific features. But portable C++11 solutions also are welcome.

Source Code:

// ItemContainer.h ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// template <class Impl> class ItemContainer { public: // Common functions supported by all specializations void Clear(); bool IsEmpty() const; ... // Functions supported by sequenced specializations only ItemPtr operator[](size_t i_index) const; ... // Functions supported by indexed specializations only ItemPtr operator[](const PrimaryKey& i_key) const; ... // Functions supported by sorted specializations only void Sort(); ... private: boost::scoped_ptr<Impl> m_data; ///< Internal container implementation }; // class ItemContainer // Forward declarations for pimpl classes, they are defined in ItemContainer.cpp struct SequencedImpl; struct IndexedImpl; struct SortedImpl; // Typedefs for specializations that are explicitly instantiated typedef ItemContainer<SequencedImpl> SequencedItemContainer; typedef ItemContainer<IndexedImpl> IndexedItemContainer; typedef ItemContainer<SortedImpl> SortedItemContainer; // ItemContainer.cpp ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Implementation classes definition, skipped as non-relevant struct SequencedImpl { ... }; struct IndexedImpl { ... }; struct SortedImpl { ... }; // Explicit instantiation of members of SequencedItemContainer template void SequencedItemContainer::Clear(); // Common template bool SequencedItemContainer::IsEmpty() const; // Common template ItemPtr SequencedItemContainer::operator[](size_t i_index) const; // Specific // Explicit instantiation of members of IndexedItemContainer template void IndexedItemContainer::Clear(); // Common template bool IndexedItemContainer::IsEmpty() const; // Common template ItemPtr IndexedItemContainer::operator[](const PrimaryKey& i_key) const; // Specific // Explicit instantiation of members of SortedItemContainer template void SortedItemContainer::Clear(); // Common template bool SortedItemContainer::IsEmpty() const; // Common template void SortedItemContainer::Sort(); // Specific // Common functions are implemented as main template members template <class Impl> bool ItemContainer<Impl>::IsEmpty() const { return m_data->empty(); // Just sample } // Specialized functions are implemented as specialized members (partial specialization) template <> void SortedItemContaner::Sort() { std::sort(m_data.begin(), m_data.end(), SortFunctor()); // Just sample } ... // etc 
2
  • 2
    What problem are you trying to solve that isn't handled by the standard C++ way of separating algorithms from data? The problem appears to be a class that has too large of an interface. Commented Sep 27, 2012 at 20:11
  • 2
    @MarkB The problem is interface with about 10 common and specialized functions that doesn't worth to split into 5 interfaces with 1-2 functions. I would like to not multiply such entities. Commented Sep 27, 2012 at 20:32

4 Answers 4

4

If it is known at compile time that a certain function will not be implemented, then that function shouldn't have been declared in the first place. Otherwise, this is a programming error.

Having said that, you must avoid declaring such a function, or declare it such that the declaration only works if it will be implemented. That can be achieved either by a static_assert or by SFINAE. For example

template<class Container> // you need one instantination per container supported struct container_traits { static const bool has_sort; // define appropriately in instantinations /* etc */ }; template<class container> class ContainerWrapper { unique_ptr<container> _m_container; template<bool sorting> typename std::enable_if< sorting>::type _m_sort() { _m_container->sort(); } template<bool sorting> typename std::enable_if<!sorting>::type _m_sort() { static_assert(0,"sort not supported"); } public void sort() { _m_sort<container_traits<container>::has_sort>(); } /* etc */ }; 
Sign up to request clarification or add additional context in comments.

2 Comments

Aha, SFINAE. Forgot about it. Not very elegant, but it works. Thanks :-)
Looks like this is the best match. I will accept it if nobody will propose something better today. Thanks!
3

Consider this example:

class A { public: void foo() {} void bar(); }; 

Only during linking phase it could be detected an error that A::bar() is not defined and this has nothing to do with templates.

You shall define separate interfaces for your different containers and use them for your implementations. Just one of the possibilities below:

template <class Impl> class ItemContainerImpl { public: ItemContainerImpl(); protected: boost::scoped_ptr<Impl> m_data; ///< Internal container implementation }; // No operations template <class Impl> class Empty : protected virtual ItemContainerImpl<Impl> {}; template <class Impl, template <class> class Access, template <class> class Extra = Empty> class ItemContainer : public Extra<Impl>, public Access<Impl> { public: // Common functions supported by all specializations void Clear(); bool IsEmpty() const; ... }; template <class Impl> class SequencedSpecialization : protected virtual ItemContainerImpl<Impl> { public: // Functions supported by sequenced specializations only ItemPtr operator[](size_t i_index) const; ... }; template <class Impl> class IndexedSpecialization : protected virtual ItemContainerImpl<Impl> { public: // Functions supported by indexed specializations only ItemPtr operator[](const PrimaryKey& i_key) const; ... }; template <class Impl> class Sorted : protected virtual ItemContainerImpl<Impl> { public: // Functions supported by sorted specializations only void Sort(); ... }; // Typedefs for specializations that are explicitly instantiated typedef ItemContainer<SequencedImpl, SequencedSpecialization> SequencedItemContainer; typedef ItemContainer<IndexedImpl, IndexedSpecialization> IndexedItemContainer; typedef ItemContainer<SortedImpl, IndexedSpecialization, Sorted> SortedItemContainer; 

5 Comments

Well, that's exactly what I'm trying to avoid - a lot of classes with 1-2 functions defined.
See last 3 lines of my answer. I am using only one ItemContainer - the rest are just helper templates to make this works. The number of functions you have to define will not change.
I understand this and considered such variant too. E.g. boost::multi_index uses similar approach. But hoping that more simple way exists.
Thanks, I have strong feeling that this approach could even reduce number of your lines of code. I believe (not sure) that you can define template classes functions in header.
I would like to keep all functions in .cpp and make header as small and simple as possible hiding all implementation details. It will be included into many files and projects and I'm not pleased to rebuild zillions of LOCs after fixing 1 LOC in header...
2

Despite the good answers suggesting to use SFINAE, I continued to search solution that will meet my original design. And finally I found it.

The key idea is to use specialization for specific function members instead of explicit instantiation.

What was done:

  1. Added specific functions dummy implementation for main template. Implementations containing static asserts only were placed in header file but not inlined into class definition.
  2. Specific functions explicit instantiations were removed from .cpp file.
  3. Specific functions specialization declarations were added to header file.

Source Code:

// ItemContainer.h ////////////////////////////////////////////////////////////////////////////// template <class Impl> class ItemContainer { public: // Common functions supported by all specializations void Clear(); bool IsEmpty() const; ... // Functions supported by sorted specializations only void Sort(); ... private: boost::scoped_ptr<Impl> m_data; ///< Internal container implementation }; // class ItemContainer // Dummy implementation of specialized function for main template template <class Impl> void ItemContainer<Impl>::Sort() { // This function is unsupported in calling specialization BOOST_STATIC_ASSERT(false); } // Forward declarations for pimpl classes, // they are defined in ItemContainer.cpp struct SortedImpl; // Typedefs for specializations that are explicitly instantiated typedef ItemContainer<SortedImpl> SortedItemContainer; // Forward declaration of specialized function member template<> void CSortedOrderContainer::Sort(); // ItemContainer.cpp ////////////////////////////////////////////////////////////////////////////// // Implementation classes definition, skipped as non-relevant struct SortedImpl { ... }; // Explicit instantiation of common members of SortedItemContainer template void SortedItemContainer::Clear(); template bool SortedItemContainer::IsEmpty() const; // Common functions are implemented as main template members template <class Impl> bool ItemContainer<Impl>::IsEmpty() const { return m_data->empty(); // Just sample } // Specialized functions are implemented as specialized members // (partial specialization) template <> void SortedItemContaner::Sort() { std::sort(m_data.begin(), m_data.end(), SortFunctor()); // Just sample } ... // etc 

This way it works at least for VS2008.

For GCC with C++11 static_assert usage requires some trick to enable lazy template function instatiation (compiled sample):

template <class T> struct X { void f(); }; template<class T> void X<T>::f() { // Could not just use static_assert(false) - it will not compile. // sizeof(T) == 0 is calculated only on template instantiation and // doesn't produce immediate compilation error static_assert(sizeof(T) == 0, "Not implemented"); } template<> void X<int>::f() { std::cout << "X<int>::f() called" << std::endl; } int main() { X<int> a; a.f(); // Compiles OK X<double> b; b.f(); // Compilation error - Not implemented! } 

5 Comments

This solution is in no way better (or worse) than a SFINAE solution. Both obtain the same goal: a compile time error. The static_assert produces a neater compiler error, but the SFINAE solution gives more readable code (the definition of your generic class ItemContainer contains no hint that Sort() will not generally compile).
@Walter No, it presents, see the end of header: template<> void CSortedOrderContainer::Sort(); I don't need to specify entire template for single member specialization.
The SFINAE constructs may look cumbersome for those who don't know them, but enable_if is rather self-explanatory. Your solution, on the other hand, contains no hint (in the class definition) that member Sort() will not compile for certain template arguments of the class template. It is thus much less clear to most human readers.
Btw, BOOST_STATIC_ASSERT is really awful, as it does not generate a nice or even useful error message. Why using such an awful macro, if the language provides a static_assert?
@Walter I know and use SFINAE, but it still looks ugly and hard readable for me. In production code we have a detailed Doxygen comments for each function declaration, so it will be clearly explained. BOOST_STATIC_ASSERT is used because it's for VS2008/C++03 which doesn't have static_assert yet. Error message is just replaced with comment right before static assert. The best we can have with C++03 :-(
1

What about this ?

template <class T, class supported_types> struct vec_enabler : boost::mpl::contains<supported_types, T> {}; // adding Sort interface template <class T, class enabler, class Enable = void> struct sort_cap{}; template <class T, class enabler> struct sort_cap<T, enabler, typename boost::enable_if< typename enabler::type >::type> { void Sort(); }; // adding operator[] template <class T, class U, class R, class enabler, class Enable = void> struct index_cap{}; template <class T, class primary_key, class ret, class enabler> struct index_cap<T, primary_key, ret, enabler, typename boost::enable_if< typename enabler::type >::type> { ret operator[](primary_key i_index) const; }; template <class Impl> class ItemContainer : public sort_cap<Impl, vec_enabler<Impl, boost::mpl::vector<A, B> > >, // sort for classes A or B public index_cap<Impl, size_t, ItemPtr, vec_enabler<Impl, boost::mpl::vector<C> > >, // index for class C public index_cap<Impl, primaryKey, ItemPtr, vec_enabler<Impl, boost::mpl::vector<B> > > // index for class B { public: void Clear(); bool IsEmpty() const; }; 

I find that using inheritance is the most clean way to achieve what you would like to do (which is 'adding interfaces to a class'.) Then we have the following:

int main(){ ItemContainer<A> cA; cA.Sort(); //ItemPtr p = cA[0]; // compile time error ItemContainer<C> cC; //cC.Sort(); // compile time error ItemPtr p = cC[0]; //ItemPtr pp= cC[primaryKey()]; // compile time error } 

Of course, you still are able to write the implementation in .cpp files.

3 Comments

Looks like you overcomplicated things a bit. I don't need different capabilities for different item types. Actually I ever don't have item type as container template parameter :-)
Indeed, but you wanted the number of classes to be limited :) (so I factorized some code.) A, B and C should be replaced by SequencedImpl, IndexedImpl SortedImpl (but I did not get into the details of which class should implement what,) so there is no different capabilities for different item types. vec_enabler is something that answers true if type T is inside Seq, false otherwise, and is used for SFINAE. index_cap: the signature is the same for both accessors, so I used the same template.
Finally sort_cap<Impl, vec_enabler<Impl, boost::mpl::vector<A, B> > > adds sorting capabilities if Impl is either A or B (same for index_cap.)

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.