Рефакторинг оператора сравнения для std :: set, использующего Dynami c Cast - PullRequest
Новая
       4

Рефакторинг оператора сравнения для std :: set, использующего Dynami c Cast

0 голосов
/ 16 января 2020

Я унаследовал кусок кода, который делает что-то вроде этого ниже. Какой самый чистый способ реорганизовать это без использования dynamic_cast для проверки того, что этот тип одинаковый?

Я мог бы пометить каждый дочерний тип значением enum, но, конечно, должно быть что-то лучше этого? 

// Example program
#include <iostream>
#include <string>
#include <set>

class A
{
public:
    virtual ~A() {}
    virtual std::string GetField() const = 0;
};

class B: public A
{
public:
    B(const std::string& i): m_field(i) {}
    virtual std::string GetField() const { return m_field; }
    std::string m_field;
};

class C: public A
{
public:
    C(const std::string& i): m_field(i) {}
    virtual std::string GetField() const { return m_field; }
    std::string m_field;
};

class LessCmp: public std::binary_function<A*, A*, bool>
{
public:
    bool operator()(A* a1, A* a2) const {
        B* b1 = dynamic_cast<B*>(a1);
        B* b2 = dynamic_cast<B*>(a2);
        if( b1 && b2 )
            return b1->GetField() < b2->GetField();
        C* c1 = dynamic_cast<C*>(a1);
        C* c2 = dynamic_cast<C*>(a2);
        if( c1 && c2 )
            return c1->GetField() < c2->GetField();
        return true;
    }
};


int main()
{
  std::set<A*, LessCmp> myset;
  myset.insert(new B("TOTO"));
  myset.insert(new B("TITI"));
  myset.insert(new B("TOTO"));
  std::cout << myset.size() << "!\n";

  myset.insert(new C("TOTO"));
  std::cout << myset.size() << "!\n";  
}
// this outputs 2 then 3
// it basically checks the GetField() order only when the types are identical

Ответы [ 2 ]

3 голосов
/ 16 января 2020

Как сказано в моем комментарии выше, я думаю, что вы вызываете UB, так как компаратор не является рефлексивным (имеет входные значения st и a<b и b<a). Вы можете обойти это, выполнив лексикографическое сравнение c для typeid и GetField: 

return std::tie(std::type_index(typeid(*a1)), a1->GetField) < std::tie(std::type_indey(typeid(*a2)), a2->GetField());
3 голосов
/ 16 января 2020

Следующее действует точно так же, как вы пробовали с помощью приведений: 

return typeid(*a1) == typeid(*a2) && a1->GetField() < a2->GetField();

, но охватывало бы, возможно, еще существующий дополнительный дериват D из A.

Но так как n314159 , обозначенный в комментариях, у нас есть еще некоторые проблемы: 

A* a1 = new B("ad");
A* a2 = new C("ad");

Теперь оба 

LessCmp()(a1, a2)
LessCmp()(a2, a1)

дадут false. Таким образом, множество будет интерпретировать их эквивалентно! Мы можем исправить это, хотя: 

std::type_index ti1(typeid(*a1));
std::type_index ti2(typeid(*a2));
return ti1 < ti2 || ti1 == ti2 && a1->GetField() < a2->GetField();

Хотя у меня есть некоторые сомнения относительно этого дизайна.

Сначала у вас есть утечка памяти: что происходит со вторым B("TOTO") экземпляр после того, как не вставить? (Хорошо, ваш исходный код может отличаться, но, как и следовало ожидать, есть утечка ...).

Тогда что отличает B("TOTO") от C("TOTO"), но не от другого B("TOTO")? Я не могу судить, так как не знаю конкретного случая использования, но он оставляет неприятные ощущения в желудке ...

...