begin() vs rbegin(): how can I avoid duplication?

Can the display function be re-written to avoid duplication of the std::for_each loop?

#include 
#include 
#include 

struct Widget
{
    int id;
    Widget(int i) : id(i){}
};

namespace std
{
    template<>
    struct less {
    public:
        bool operator ()(Widget *const& a, Widget *const& b) const {
            return  (a->id id);
        }
    };
}

void display(const std::set& wList, bool order)
{
    if (order) {
        std::for_each(wList.begin(), wList.end(), [](const Widget* x) {
            std::cout id id  s;

    s.insert(new Widget(1));
    s.insert(new Widget(2));
    s.insert(new Widget(5));
    s.insert(new Widget(3));
    s.insert(new Widget(9));
    s.insert(new Widget(7));

    display(s, false);
}

I would re-write the display to take iterators.

template
void display(I begin, I end)
{
    std::for_each(begin, end, [](const Widget* x) {
         std::cout id << std::endl;
    });
}

Then you can pass any container that contains Widget pointers to the method. Also solves the problem about ordering. You pass the appropriate iterators for the task.

But my main problem. Is why are you storing pointers in a container?

Who owns the pointers? If it is the container then you are doing it wrong as the destructor will not clean up (nor will replacing elements do the correct thing). For this you should use boost::ptr_set (in `) or std::set> if the container is not taking ownership then it should hold references to the object std::set>` there are other alternatives but it depends intirely on the semantics you intend to achieve.

But pointers in containers is a no-no (in general). Better have a darn good reason.

StackExchange Code Review Q#61301, answer score: 6