compiler error with function to sort vector of objects on member variable value

[LMHmedchem]

Hello,

I tried to set up a function to sort a vector of objects like I did in this thread,
http://forums.codeguru.com/showthrea...63#post2170563

I am getting a compiler error.

In function `void igroups_cppfunc__(float*, int*)':
src/src_client_main/igroup_cfunc.cpp:1940: error: no match for 'operator&' in '(&dispersion_subgraph_list)->std::vector<_Tp, _Alloc>::end [with _Tp = dispersion_subgraph, _Alloc = std::allocator<dispersion_subgraph>]() & dispersion_object_sort'
/usr/lib/gcc/i686-pc-mingw32/3.4.4/include/c++/bits/ios_base.h:80: note: candidates are: std::_Ios_Fmtflags std::operator&(std::_Ios_Fmtflags, std::_Ios_Fmtflags)
/usr/lib/gcc/i686-pc-mingw32/3.4.4/include/c++/bits/ios_base.h:120: note: std::_Ios_Openmode std::operator&(std::_Ios_Openmode, std::_Ios_Openmode)
/usr/lib/gcc/i686-pc-mingw32/3.4.4/include/c++/bits/ios_base.h:158: note: std::_Ios_Iostate std::operator&(std::_Ios_Iostate, std::_Ios_Iostate)

The code is as follows,
In header_file.h

// class for information for each identified dispersion_subgraph
class dispersion_subgraph {

public:
   // initialize class members
   dispersion_subgraph()
      : disp_subgraph_count(0), substituent_count(0), subgraph_AtR(0.0), relative_subgraph_AtR(0.0) { }

   // class member variables
   int disp_subgraph_count, substituent_count;
   float sum_substituent_SCEstate, relative_subgraph_AtR;
   vector<int> substituent_atomNum;
   vector<float> substituent_SCEstate;
};

// prototype for sorting function
bool dispersion_object_sort(const dispersion_subgraph & lhs, const dispersion_subgraph & rhs);

In src.cpp

// sort vector of dispersion objects large to small on float relative_subgraph_AtR
bool dispersion_object_sort(const dispersion_subgraph & lhs, const dispersion_subgraph & rhs) {

return lhs.relative_subgraph_AtR > rhs.relative_subgraph_AtR;
}

// vector of dispersion_subgraph objects
vector<dispersion_subgraph> dispersion_subgraph_list;

// sort vector of result objects on relative_subgraph_AtR, large to small
sort(dispersion_subgraph_list.begin(), dispersion_subgraph_list.end() &dispersion_object_sort);

The compiler error is at the last line above. I know I have used this code before so I am surprised that I can't get it working. I don't see anything obviously wrong with the code, so I am at a bit of a loss.

Suggestions would be greatly appreciated. This is with gcc 3.4.4, so possibly this solution was based on a later version.

LMHmedchem

[LMHmedchem]

Well it looks like I was just missing a comma after dispersion_subgraph_list.end() in the last line. At least it compiles when I add the comma back in. That was pretty dumb of me to miss.

Other than my not being able to read, is there anything else wrong with this approach?

Is there a way to make a more generic sorting function where I could specify which member variable to sort on in the call?

LMHmedchem

[superbonzo]

Other than my not being able to read, is there anything else wrong with this approach?

just the naming seems a bit misleading ( dispersion_object_sort suggests a sorting function at first sight; dispersion_object_comparator may sound better ... )

Is there a way to make a more generic sorting function where I could specify which member variable to sort on in the call?

sure you can(*), not so sure it's worth it, though; if you need to compare against so few variables it may be simpler/more readbale/mantainable to define a comparator for each of them ...

(*) there are many ways of doing it, each with its pros and cons ... a c++14 example might be ( a c++03 version is also possible )

template < typename T, typename F >
struct greater_by_comparator
{
    F fetcher;

    bool operator()( T const& lhs, T const& rhs )
    {
        return fetcher(lhs) > fetcher(rhs);
    }
};

template < typename T, typename F >
auto greater_by( F fetcher ){ return greater_by_comparator<T,F>{ fetcher }; }

template < typename T, typename V >
auto greater_by( V T::* member ){ return greater_by<T>( [=]( auto const& v ){ return v.*member; } ); }

// to be used as

sort(dispersion_subgraph_list.begin(), dispersion_subgraph_list.end(), greater_by( &dispersion_subgraph::relative_subgraph_AtR ) );

// or, if you want more freedom ( but this is probably not better than writing the predicate lambda directly; it may be still useful with generic code ... )

sort(dispersion_subgraph_list.begin(), dispersion_subgraph_list.end(),
    greater_by<dispersion_subgraph>( []( auto const& d ){ return d.relative_subgraph_AtR; } ) );

as said, there are many ways of doing it ...

[superbonzo]

and here is a (less generic) c++03 version:

template < typename T, typename V >
struct greater_by_comparator
{
    greater_by_comparator( V T::*member ): member_(member){}
        
    bool operator()( T const& lhs, T const& rhs )
    {
        return (lhs.*member_) > (rhs.*member_);
    }
    
private:
    V T::*member_;
};

template < typename T, typename V >
greater_by_comparator<T,V> greater_by( V T::* member ){ return greater_by_comparator<T,V>( member ); }

// to be used as

sort(dispersion_subgraph_list.begin(), dispersion_subgraph_list.end(), greater_by( &dispersion_subgraph::relative_subgraph_AtR ) );

[LMHmedchem]

Thanks for the information. I always seem to end up with some issue where I need to sort vectors of objects and I am beginning to think I should just plan on having an object sorting function with the classes I create.

Would there be some logic to this being a class member function as part of the class definition? If so, what would that look like in the class definition and in usage?

Thanks,

LMHmedchem

[superbonzo]

I am beginning to think I should just plan on having an object sorting function with the classes I create.

that makes perfect sense, as long as the ordering is inherently "part" of the class meaning; otherwise, it's better to define them locally to avoid their uncontrolled proliferation and consequent code pollution, IMO

Would there be some logic to this being a class member function as part of the class definition? If so, what would that look like in the class definition and in usage?

as a member function you can write a '<' operator; but, this makes sense only if there's some sort of natural ordering, otherwise it's generally not a good idea. As said, there are gazillion ways of "attaching" an ordering to a type at different abstraction levels.

anyway, the simplest/cleanest way to attach a set of "often used" orderings associated to some type foo, is to put them in one or more dedicated namespaces:

// foo.h

class foo { ... }; // the class

// foo_ordering.h

namespace foo_ordering // this shall be grouped in a meaningful way, say, as belonging to the same "usage domain"
{

bool by_bar( foo const&, foo const& ) { ... }
bool by_foobar( foo const&, foo const& ) { ... }
...

}

// usage.cpp

#include<foo.h>
#include<foo_ordering.h>

...

sort( foos.begin(), foos.end(), foo_ordering::by_bar );

using namespace foo_ordering; // in code where is obvious that foo's have a foo_ordering

sort( foos.begin(), foos.end(), by_bar );
sort( foos.begin(), foos.end(), by_foobar );

in this way you have a separate file and namespace for each "set" of orderings ( some set may apply in some part of your programs, some not ); note, these are not friend which is a good thing ( if you need a lot of friend sorting functions, you probably have a design problem ).