Making items in a set do stuff

Take a look:

Code:

set<Circle*>* circles = new set<Circle*>() ;
	
for(int i=0; i<100; i++)
    circles->insert(new Circle(rand()%screen_width, rand()%screen_height ));
	
set<Circle*>::iterator pos;
		
for (pos = circles->begin(); pos != circles->end(); pos++) 
{
    pos->draw(buffer);
}

The Circle class has a well and functioning function named draw. The problem here is how to invoke it on all of the Circles in the set circles.

You need two indirections, one for the iterator and one for the pointer.

Code:

(*pos)->draw(buffer);

You can also use algorithms if you want to perform the same action on all objects in the container.

Do you have a particular reason for dynamically allocating the set or for holding pointers rather than objects inside it? In both cases you should generally use regular objects unless you have a specific reason to do otherwise.

Originally Posted by Daved

You need two indirections, one for the iterator and one for the pointer.

Code:

(*pos)->draw(buffer);

You can also use algorithms if you want to perform the same action on all objects in the container.

Do you have a particular reason for dynamically allocating the set or for holding pointers rather than objects inside it? In both cases you should generally use regular objects unless you have a specific reason to do otherwise.

So, I really have no reason to use pointers to objects here? Should I only do it when passing into functions and stuff?

Thanks by the way for that.

There is practically never any reason to allocate STL containers themselves dynamically.

The contents of containers are dynamically allocated if you need polymorphism (if your set might also contain sub-classes of Circle's) or if the objects are too expensive or otherwise difficult or impossible to copy.

By the way, as you don't apparently provide any special comparison function, the use of set is quite questionable. The items are ordered according to pointer addresses. Therefore the ordering will be quite random from your perspective (e.g the order in which objects were allocated) and the set won't filter out equivalent objects (because all pointers will be unique), so you might just use a vector instead? (And if it weren't so, and not all allocated objects were actually inserted into the set, you'd leak that memory.)

What would be the easiest way to pass in a comparator? I tried the function way, but I couldn't get the syntax right. And then there is the struct way... If possible, can you show me both?

>> So, I really have no reason to use pointers to objects here? Should I only do it when passing into functions and stuff?

Even passing to functions doesn't really require pointers. If you don't want to pass a copy for efficiency reasons (for example, it is probably best not to copy a full set of objects) then pass a reference to const:

Code:

void drawCircles(const std::set<Circle>& drawableCircles);

If you don't want to pass a copy because you want the actual object to be modified, then pass a plain reference:

Code:

void addDrawableCircles(std::set<Circle>& drawableCircles);

As anon pointed out there are good reasons to use pointers in containers (like for polymorphism or non-copyable objects). There are usually better solutions than using raw pointers there as well, but raw pointers are fine if you have an obvious way of cleaning them up at the right time.

>> What would be the easiest way to pass in a comparator?
It would be easier for you to show your attempt, can you post that and the errors you got? I just recently gave an example of a struct that you can search for if you have found an example already.

Code:

struct circlecomparator
{
  bool operator()(Circle* c, Circle* i) //const
  {
    return c->gsize() >= i->gsize();
  }
};

//...

set<Circle*,circlecomparator>* circles = new set<Circle*>() ;

In the example I saw there was a const after the function, but I commented it out, as I didn't understand it.

I do this, and I get an error:
cannot convert `std::set<Circle*, std::less<Circle*>, std::allocator<Circle*> >*' to `std::set<Circle*, circlecomparator, std::allocator<Circle*> >*' in initialization

It just asked for a struct that had the () operator overloaded.

Leave the const in, it means that the function doesn't change the state of the struct (which is obviously true since the struct has no state).

Look at the difference between the declaration of the circles pointer (it is set<Circle*,circlecomparator>) and the type you pass to new (which is a set<Circle*>). They should be the same. Of course, since you probably don't need new here you can just do this:

Code:

set<Circle*,circlecomparator> circles;

One other issue with your comparison function is that it returns true for equal. Compare functions should provide a strict weak ordering, which includes the rule that a == b if and only if !(a.compare(b)) && !(b.compare(a)). Use > instead of >= to solve that.

Oh, right, being careless. Would it be easier if I used a function to compare? I guess it wouldn't change too much. Thanks again.

It would be easiest if you could use the default comparison function, that is the operator<. Note that pointers can be compared with < but most likely it is not what you need.

If you need to use a different comparison function / function object, typedef's can help (because the set would otherwise have very long template arguments. With a user-defined function it might look like this:

Code:

bool compare(int a, int b)
{
    return b < a;
}

int main()
{
    typedef std::set<int, bool(*)(int, int)> IntSet;
    IntSet s(compare);
    ...
}

If you have sets of different kinds it might also help to templatize the functions that work on sets.

Code:

#include <iostream>
#include <set>
#include <cstdlib>

bool descending(int a, int b)
{
    return b < a;
}

//this function works on any kind of set (ignoring the allocator)
template <class Key, class Compare>
void print_set(const std::set<Key, Compare>& s)
{
    for (typename std::set<Key, Compare>::const_iterator it = s.begin(); it != s.end(); ++it)
        std::cout << *it << ' ';
    std::cout << '\n';
}

int main()
{
    typedef std::set<int, bool(*)(int, int)> IntSet;
    IntSet s1(descending);

    //another set uses default for comparison
    std::set<int> s2;

    for (int i = 0; i != 20; ++i) {
        int r = rand() &#37; 32;
        s1.insert(r);
        s2.insert(r);
    }

    //same function can handle both sets
    print_set(s1);
    print_set(s2);
}