1

When I try to compile the following code:

 #include <iostream> #include <set> #include <vector> using namespace std; template <class T, class S> class Property { public: pair<T,S> p; Property(T t, S s) { p = make_pair(t,s);} }; int main() { set< Property<string, string> > properties; Property<string, string> name("name", "Andy"); properties.insert(name); }

I get the compilation error. However, when I replace set by vector and hence use the the push_back function instead of insert function everything works fine. Could anyone explain me what am I doing wrong? Thanks in advice.

Improve this question
4
  • You might want to try std::unordered_set if you don't care about the ordering, which you don't seem to here, judging by the error you get, which I've magically deciphered from compiling this in my brain. Commented Feb 9, 2013 at 3:34
  • @chris it'd probably be more work to use std::unordered_set since you have to provide a hash and equality operator. Commented Feb 9, 2013 at 3:44
  • @Rapptz, True, though ordering doesn't seem to me like it would be prominent here. Commented Feb 9, 2013 at 3:46
  • Without defining ordering (operator <(const Property&), the standard std::set has no way of determining identity of an element (which, btw, is done with (!(left < right) && !(right < left)), so make sure your order is consistent). The std::pair<> template defines the most commonly used order comparator already, so unless you have a specific reason not to, you could well easily solve your issue by simply throwing out Property as a formal definition and instead just using typedef std::pair<string,string> Property; I'm guessing, however, you need case-insensitive label comparison. Commented Feb 9, 2013 at 3:46

2 Answers 2

Reset to default
5

std::set stores its values in a sorted binary tree, so it needs to know how to compare the values it holds. By default it uses std::less as a comparison function, which for un-specialized user defined types tries to call operator<. So, the easiest way to tell the set how to compare your objects is to define an operator< for your class:

template <class T, class S> class Property { public: pair<T,S> p; Property(T t, S s) { p = make_pair(t,s);} bool operator<(const Property<T,S>& rhs) const { return p < rhs.p; } };

However, there are also other ways of telling std::set how to compare your type. One is to specialize the std::less template for your class:

namespace std { template<typename T,typename S> struct less<Property<T, S> > { bool operator()(const Property<T, S>& lhs, const Property<T,S>& rhs) const { return lhs.p < rhs.p; } }; }

Another is to replace the default comparison type with a function with the correct signature, or a class that has an operator() defined with the correct signature. This is where things start to get ugly.

// Comparison function template<typename T, typename S> bool property_less_function(const Property<T,S>& lhs, const Property<T,S>& rhs) { return lhs.p < rhs.p; } // Comparison functor template<typename T, typename S> struct PropertyLess { bool operator()(const Property<T,S>& lhs, const Property<T,S>& rhs) const { return lhs.p < rhs.p; } }; int main() { // Set using comparison function. // Have to pass the function pointer in the constructor so it knows // which function to call. The syntax could be cleaned up with some // typedefs. std::set<Property<std::string, std::string>, bool(*)(const Property<std::string, std::string>&, const Property<std::string, std::string>&)> set1(&property_less_function<std::string, std::string>); // Set using comparison functor. Don't have to pass a value for the functor // because it will be default constructed. std::set<Property<std::string, std::string>, PropertyLess<std::string, std::string> > set2; }

Keep in mind that whatever less-than function you use, that function must define a strict weak ordering for your type.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I'd move the template from PropertyLess to it's operator(), that way you don't have to duplicate the parameters std::set<Property<std::string, std::string>, PropertyLess> set3;
2

In order to insert something into std::set, you need to have operator< defined.

For example this compiles fine on GCC 4.7.2:

#include <iostream> #include <set> #include <vector> using namespace std; template <class T, class S> class Property { public: pair<T,S> p; Property(T t, S s) { p = make_pair(t,s); } bool operator<(const Property& p2) const { //Something naive.. return p < p2.p; } }; int main() { set< Property<string, string> > properties; Property<string, string> name("name", "Andy"); properties.insert(name); }

An alternative would be to use std::unordered_set though that would require you to provide a hash for the key and defining operator==.

Improve this answer

2 Comments

Your implementation of operator< does not give a strict weak order, so it will cause problems at runtime. Just use pair's operator< to directly compare the stored pairs.
You don't need operator<, you could also provide a comparitor.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.