Sort function is driving me nuts

[TheWolverine]

Hi all,

I've spent the last week trying to sort this out, and the more I delve into it, the more confused I'm getting. I've done some background research (searched on google and here for sort function issues) and really I can't make head or tail of what's going on.

My question is quite simple. Why does the following not work?

Let's say I have a class Foo with two members:

#include "random.h"

class Foo
{
public:
    double fooMember1;
    double fooMember2;
    Random rand;

    Foo(){};
    ~Foo(){};

    void setFooMemberValues()
    {
        //random value generated with a random generator
        fooMember1 = rand.getRandomNumber(); 
        //random value generated with a random generator
        fooMember2 = rand.getRandomNumber(); 
    };
};

Now let's say I have another class called Foo2:

#include "foo.h"
#include <vector>
#include <algorithm>

class Foo2
{
public:
    std::vector<Foo*> fooObjectPointers;

    void run()
    {
        createFoo();
        sort(fooObjectPointers.begin(), fooObjectPointers.end(), fooSort);
        destroyAllFoo();
    };

    void createFoo()
    {
        for( int i=0; i<10; i++ )
            fooObjectPointers.push_back( new Foo );
    };

    void destroyAllFoo()
    {
        for( unsigned int i=0; i<fooObjectPointers.size(); i++ )
            delete fooObjectPointers[i];
    };

    bool fooSort(Foo* fooPointer1, Foo* fooPointer2)
    {
        return fooPointer1->fooMember1 < fooPointer2->fooMember2;
    };

};

This gives me the following compiler error:

:foo2.cpp:20: error: no matching function for call to 'sort(__gnu_cxx::__normal_iterator<Foo**, std::vector<Foo*, std::allocator<Foo*> > >, __gnu_cxx::__normal_iterator<Foo**, std::vector<Foo*, std::allocator<Foo*> > >, <unknown type>)'

So I googled this, and discovered that it doesn't work because the comparison function fooSort is non-static (this all works fine if I just declare the function separate from the class Foo2 and do the sorting in main()).

I didn't really and still don't understand what it means and the more I delved into it the more I came across things I didn't understand (like functors).

I found this workaround in the end: http://bytes.com/topic/c/answers/525611-problems-stl-sorting-using-member-functions.

When I employed this at first, it seemed everything was working fine (no idea why, but ok it seemed to be working). Then I decided to check for sure and I printed out the values of fooMember1 in each of the objects pointed to by the entries in the vector fooObjectPointers. I discovered to my surprise that the values of fooMember1 had completely changed! Yes, they were now all sorted correctly, but somehow the values were totally different! What's more, the values of fooMember2 were also completely different.

Somehow it seems as though new objects of Foo were instantiated through the sorting procedure, by which fooMember1 and fooMember2 acquired new random values. The sorting proceeded with these values and NOT the original values that I wanted to sort.

Then I decided that maybe it would be easiest to just program my own sort function, so I went ahead and did this. I discovered to my amazement that the same thing was happening. In particular, the following assignment seems to be doing something nuts in my code (nuts to me at least):

std::vector<Foo*> fooObjectPointers1;
std::vector<Foo*> fooObjectPointers2;
fooObjectPointers2 = fooObjectPointer1;

Here, fooObjectPointers1 points to objects that I created earlier using the createFoo() function. fooObjectPointers2 has yet to be filled. When I did the "=" assignment in main() and I print out the vectors, the memory addresses are exactly the same, and indeed they point to the same objects when I print out the member variables of Foo. When I do this "=" assignment within the Foo2 class, fooObjectPointers2 has a different set of memory addresses, and points to totally different values of the member variables of Foo!!!

Also, to complicate my question more, why can I not do this?

std::vector<Foo*> fooObjectPointers1;
std::vector<Foo*> fooObjectPointers2;
fooObjectPointers2[0] = fooObjectPointer1[0];

Program builds fine but crashes when I run it when I do that.

I'm guessing I'm doing something very stupid and basic, but I just can't figure it out.

I'd appreciate any and all help. This is something I really need to sort out at short notice because I can proceed with the rest of my code.

Thanks in advance,

Kartik

[dusktreader]

Hi all,

I've spent the last week trying to sort this out, and the more I delve into it, the more confused I'm getting. I've done some background research (searched on google and here for sort function issues) and really I can't make head or tail of what's going on.

The sort function actually takes a struct as the third argument. The struct has a funciont pointer for a comparison function. See this example, implement it yourself, and then try to adapt it: c++ slt sort.

I know the sort interface is clunky, but it will work if you set it up correctly. The convenience after that is very nice

[Fbody]

Did you even read that link you posted? It doesn't have to be a struct, but you can use one that has a bool function in it. All you really need is a function that returns true when the items being compared are in proper order.

[dusktreader]

Did you even read that link you posted? It doesn't have to be a struct, but you can use one that has a bool function in it. All you really need is a function that returns true when the items being compared are in proper order.

Oops. You are correct. I should have read the link more carefully.

[Fbody]

@OP:
try adding the keyword const to your parameters for fooSort().

bool fooSort(const Foo* fooPointer1, const Foo* fooPointer2)

Also, I believe you need to pull sort into the std namespace like you did with vector.

[TheWolverine]

@OP:
try adding the keyword const to your parameters for fooSort().

bool fooSort(const Foo* fooPointer1, const Foo* fooPointer2)

Also, I believe you need to pull sort into the std namespace like you did with vector.

Ah yea, I forgot pulling sort from std. I tried adding the word const but to no avail. Throwing this error:

simulation.cpp:20: error: no matching function for call to 'sort(__gnu_cxx::__normal_iterator<Foo**, std::vector<Foo*, std::allocator<Foo*> > >, __gnu_cxx::__normal_iterator<Foo**, std::vector<Foo*, std::allocator<Foo*> > >, <unknown type>)'

I really don't get what the issue is. Anyway more feedback would be greatly appreciated.

Thanks,

Kartik

[mitrmkar]

Two options to make this sort(...) work, would be

Make the fooSort() a static member function, so ..

[I]static[/I] bool fooSort(const Foo * fooPointer1, const Foo * fooPointer2)
    {
        return fooPointer1->fooMember1 < fooPointer2->fooMember2;
    }

or take the fooSort() function outside of the class completely, so ..

bool fooSort(const Foo * fooPointer1, const Foo * fooPointer2)
{
    return fooPointer1->fooMember1 < fooPointer2->fooMember2;
}

The above is based on what you posted in your initial post. But I hope you get the idea.
Then a couple of things..
- there is a member variable Random [I]rand[/I] , that is bad, since the standard library comes with a routine named rand() , so try to avoid name clashes.
- You are typing in extra semicolons as in

void setFooMemberValues()
{
<snip>
} ;

Depending on the mode you compile with GCC, your code might be totally rejected.

Then you asked what is wrong with the following ..

std::vector<Foo*> fooObjectPointers1;
std::vector<Foo*> fooObjectPointers2;
// The vectors contain nothing at this point, trying to access
// the non-existent element causes the crash
fooObjectPointers2[0] = fooObjectPointer1[0];

A safer way to try to access would be fooObjectPointers2.at(0) , which would give you an exception that could be caught.

[TheWolverine]

Two options to make this sort(...) work, would be

Make the fooSort() a static member function, so ..

[I]static[/I] bool fooSort(const Foo * fooPointer1, const Foo * fooPointer2)
    {
        return fooPointer1->fooMember1 < fooPointer2->fooMember2;
    }

or take the fooSort() function outside of the class completely, so ..

bool fooSort(const Foo * fooPointer1, const Foo * fooPointer2)
{
    return fooPointer1->fooMember1 < fooPointer2->fooMember2;
}

The above is based on what you posted in your initial post. But I hope you get the idea.
Then a couple of things..
- there is a member variable Random [I]rand[/I] , that is bad, since the standard library comes with a routine named rand() , so try to avoid name clashes.
- You are typing in extra semicolons as in

void setFooMemberValues()
{
<snip>
} ;

Depending on the mode you compile with GCC, your code might be totally rejected.

Then you asked what is wrong with the following ..

std::vector<Foo*> fooObjectPointers1;
std::vector<Foo*> fooObjectPointers2;
// The vectors contain nothing at this point, trying to access
// the non-existent element causes the crash
fooObjectPointers2[0] = fooObjectPointer1[0];

A safer way to try to access would be fooObjectPointers2.at(0) , which would give you an exception that could be caught.

Thanks for your feedback.

I tried making the function static. It solves the compiler error but the end result is that the vector does not get sorted. I output the contents of the vector and basically the sort function does absolutely nothing to the vector it seems.

Then I tried to remove fooSort from the class but when I try to then use the sort function within the class, referencing fooSort, it says (logically) that the function fooSort is not defined within scope of the sort command.

Thanks for spotting the rand variable declaration, silly mistake. Also thanks for pointing out the extra semi-colons.

Maybe by making fooSort static I'm effectively freezing it and it's just not applying anything to the vector fooObjectPointers?

Thanks,

Kartik

[mitrmkar]

Perhaps you could post the code you've tried?

[TheWolverine]

Perhaps you could post the code you've tried?

It works! I tried to code it up again and I guess the problem was that I was declaring the function static in the wrong place. I've declared it static in the header file now and it seems to be working fine! :)

One week of headache and all it comes down to is adding the word "static"!

Anyway, I hope I don't discover that there are still issues with it. For now, my output seems to be sorted fine. So fingers crossed it's solved!

Thanks a lot!!!

If there's any issue with it I'll let you know, but for now looks good! :)

Kartik

[dusktreader]

As a post note, the sort function will also work on user defined classes without a third parameter (comparison function) if the class has overloaded the < operator.