1

I am trying to sort a vector that has an object of a derived class, where the base class have protected members

In my header I have the base class:

class Item{
protected:
    int ID;
    string name;
    int cost;
    int sell;
    int profit;
    float profitperh;
    int time;               // in seconds!
    vector<string> usedFor;

public:
    //getters
    //setters
    //functions
    virtual bool DescProfit (const Item& i1, const Item& i2) const;
    bool operator < (const Item& i1) const;
}

From Base class I derive a couple of objects, the one I am currently working with to see if it works fine is named Seed and looks like this:

class Seed : public Item{
private:

public:
        //Constructors
virtual bool operator < (const Item& i1) const{
        return (profit < i1.Item::profit);    // Also tried this line with i1.Item::profit and i1.Item::getProfit() and i1.Item::profit
    }

//    virtual bool DescProfit (const Item& i1, const Item& i2) const{
//        return i1.profit > i2.profit;   // Also tried this line with i1.Item::profit and i1.Item::getProfit() and i1.Item::profit
//    }         //end DescProfit

When I try to build this it complains on the return line that 'profit' is a protected member of 'Item' The operator < and DescProfit are to be used with sort(), to sort the vector:

vector<Item*> v1;
sort(v1.begin(), v1.end(), DescProfit);

I've tried to place the bool function both inside and outside of class with the same error mentioned above. Does anyone have any ideas to what the error might be? I got the "template" for the above code from this post: Sorting a vector of custom objects What do I need to do to be able to access the protected variable in the base class?

Thank you in advance

Community
  • 1
  • 1
Ahana
  • 19
  • 6
  • You're vector is of `Item` **pointers**, not `Item`. Your comparator won't be called at all by-default. And that operator should be defined in the base class regardless. There are no object *pointers* in the linked question you provided. You can bet something will be different (and you'd win that bet). – WhozCraig Aug 05 '14 at 02:14
  • You declare a vector of pointers to `Item` but then try to sort it with a predicate that takes an `Item` and not a pointer to `Item`. – vsoftco Aug 05 '14 at 02:14
  • Its just a suggestion..use function objects and overload "()" operator inside that for extensibility. – cbinder Aug 05 '14 at 02:20
  • I am using polymorphism (virtual) and the comparator is therefor called depending on which type of derived function it is. I tried to remove that in this post but apparently failed to remove the pointer in the main. – Ahana Aug 05 '14 at 02:29
  • vsoftco - How do you suggest to change it when I push different kinds of derived classes into the vector? – Ahana Aug 05 '14 at 02:33
  • @Ahana First, why doesn't your class have a `int getProfit() const;` function that is `public`? That would, at least, fix your first issue. – PaulMcKenzie Aug 05 '14 at 02:40
  • @PaulMcKenzie I just tried that and it build correctly, but how would I then send it to the sort? just `v1[0]->DescProfit`? – Ahana Aug 05 '14 at 03:26
  • Does it makes sense to implement a comparison method in a derived class which does not have access to the base class data? What happens if the derived class adds data that changes the way it compares to other classes? – veefu Aug 05 '14 at 03:46
  • @veefu It's a virtual function, so depending on the derived class it will sort from different fields – Ahana Aug 05 '14 at 04:04
  • Please post `Item::operator<`. Is it virtual in your real code? – n. m. could be an AI Aug 05 '14 at 04:24
  • @n.m. Yes it is virtual in my real code – Ahana Aug 05 '14 at 05:06
  • Please edit the question so that this information is in the question itself. – n. m. could be an AI Aug 05 '14 at 05:11

2 Answers2

0

Binary operators like < and polymorphic class hierarchies don't work together well.

Even if you declare operator< virtual in Item, and override it in Seed, the other argument is an Item regardless. The overridden comparison operator can use Seed properties of this object, but only Item properties of the other object, because it is not known whether the other object is a Seed or not.

This makes defining a virtual < that works like it should (antisymmetric, and in concert with > and ==, and not violating LSP) virtually impossible.

This is true in any language, nothing specific to C++ or vectors here. Some languages offer multiple dispatch that ostensibly solves the problem. However, multiple dispatch is a closed-world solution (requires advance knowledge of all classes in the hierarchy), while normal OO is open-world. If you can live with a closed-world solution, you can do multiple dispatch in C++ too.

The best course of action is to have a non-virtual < that need not be overridden at all.

n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243
  • 1
    What about using typeid to check during runtime to see if it is of that type? Would that work? – Ahana Aug 05 '14 at 05:09
  • Typeid is bad for a number of reasons. It is OK to use it in infrastructure code, but in business code it breaks LSP and should be avoided. If you must check dynamic types, prefer dynamic_cast. It will not solve the this problem anyway because you can compare both Item with Seed and Seed with Item, and you have to make the two cases consistent. – n. m. could be an AI Aug 05 '14 at 05:18
  • This is jsut a private project of mine - but thank you so much for you're help :) Gonna work on redesign for it tomorrow – Ahana Aug 05 '14 at 06:19
0

AFAIK there is a way to access parent class protected members by using member pointers (this probably is a hole in the type system)

class Seed : public Item {

public:
    virtual bool operator < (const Item& i1) const {
        int Item::*profitptr = &Seed::profit;
        return (profit < i1.*profitptr);
    }

}

However unless it is very necessary, it is not recommended. I would recommend that you redesign your architecture so that you can avoid that.

Krypton
  • 3,337
  • 5
  • 32
  • 52
  • Thank you, I'll think of something to make it a non virtual in the base class instead, which n.m. suggested too in above answer :) – Ahana Aug 05 '14 at 05:11