bool operator() and inheritance

Question

I have the following problem with bool operator() for derived class

Base class

class Point
{
double x, y;
public:
Point(){x=0;y=0;}
...
}

Derived class

class 3DPoint : public Point
{
double z;
public:
3DPoint(double x, double y, double zx) : Point(x,y){z(zz);}
...
}

operator () for derived class

class compareByX
{
bool operator () (const 3DPoint *p1, const 3DPoint *p2) const
{
return p1->x < p2->x;   //Compilation error
}
}

Container of points

class List: public list<3DPoint *>
{
...
}


int main()
{
List l;;
l.push_back(new 3DPoint(1,2,3));
l.push_back(new 3DPoint(4,5,6));
sort(l.begin(), l.end(), compareByX);  
}

The compilation stops in class compareByX with the following message: can not convert 3DPoint const to Point. I removed const declaration...

class compareByX
{
bool operator () (3DPoint *p1, 3DPoint *p2) const
{
return p1->x < p2->x;   //Compilation error
}
}

... and... successful compilation. But I believe that operator () is not well defined. Can you help me, please? Perhaps it would be better to propose more suitable object model... Thanx.

Answer 1

• Classes and structure definitions should be followed by a ;
• Identifier names cannot start with a digit

Answer 2

Your x,y,z are private members, and you are trying to access them from outside a class. Either make your points structs, or make your x,y,z public, or provide setters/getters for them.

EDIT:
Couple more things about your code:

1. Do not inherit your class List from std::list, standard containers are not meant to be used as base classes. If you need a special function that's not available in std::container, provide a free function which can do that, instead of inheriting from it.
2. Considering the type of question here, implementing your own container is probably not the best idea. Use some standard one, there's plenty of them, and they will most likely fit your needs.
3. When inheriting one class from another, the base class should normally be virtual.
4. Point3D isn't kind of Point2D, it's more like Point2D and Point3D are kind of Point. To me this kind of inheritance would make a bit more sense.

And just to stop guessing the compiler error you have there, give a try to this code, I think it's approximately what you're looking for.

#include <algorithm>
#include <iostream>
#include <vector>

class Point
{
public:
    Point() {}
    virtual ~Point() {}

    virtual void some_function_relevant_to_all_points() {}

private:
    // maybe some members here
};

class Point2D : public Point
{
public:
    Point2D(double x, double y)
        : x_(0),
          y_(0)
    {}
    ~Point2D() {}

private:
    double x_;
    double y_;
};

class Point3D : public Point
{
public:
    Point3D(double x, double y, double z)
        : x_(x),
          y_(y),
          z_(z)
    {}
    ~Point3D() {}

    double get_x() const {return x_;}
    double get_y() const {return y_;}
    double get_z() const {return z_;}

private:
    double x_;
    double y_;
    double z_;
};

class Compare3DPointByX
{
public:
    bool operator()(const Point3D *lhs, const Point3D *rhs) const
    {
        return lhs->get_x() < rhs->get_x();
    }
};

class DeleteElement
{
public:
    template <typename T>
    void operator()(T *arg)
    {
        delete arg;
    }
};

int main()
{
    std::vector<Point3D *> points3d;
    points3d.push_back(new Point3D(4,5,6));
    points3d.push_back(new Point3D(1,2,3));

    std::cout << "point 1:" << points3d[0]->get_x() << "\n";
    std::cout << "point 2:" << points3d[1]->get_x() << "\n";

    std::sort(points3d.begin(), points3d.end(), Compare3DPointByX());

    std::cout << "point 1:" << points3d[0]->get_x() << "\n";
    std::cout << "point 2:" << points3d[1]->get_x() << "\n";

    std::for_each(points3d.begin(), points3d.end(), DeleteElement());
    return 0;
}

Rest of functionality you can add yourself, this example is just to give you the idea, how it might be implemented.

Hope it helps, good luck.

Answer 3

I correct the code (thans for checks), the same question....

class Point3D : public Point
{
double z;
public:
Point3D(double x, double y, double zx) : Point(x,y){z(zz);}
...
}

class compareByX
{
bool operator () (Point3D *p1, Point3D *p2) const
{
return p1->getX() < p2->getX();   //Compilation error
}
}


class List: public list<Point3D *>
{
...
}


int main()
{
List l;;
l.push_back(new Point3D(1,2,3));
l.push_back(new Point3D(4,5,6));
sort(l.begin(), l.end(), compareByX);  
}

Answer 4

You can't sort a std::list like that, you'll need something that supports random access like a std::vector. Either that or use the sort member function:

l.sort(compareByX());  

Note the parantheses I've added to compareByX, you need to construct a functor and that's what the parentheses are for.

Also, you have to make your operator() a public member function so that the sort algorithm can call it. The simplest way to achieve this is making your functor a struct:

struct compareByX
{
    bool operator () (Point3D const *p1, Point3D const *p2) const
    {
        return p1->getX() < p2->getX();   
    }
};

I suspect that you'll also have to make your getX member function public but that's difficult to tell because you're not showing that part of your code.

Finally, I don't think you need pointers/heap allocation for this particular example. Your program will be faster and more robust if you create your points on the stack.

PS: please post real code next time, it will make it much easier for those trying to help.

Answer 5

Thanx to all for answers and comments....