Безопасно ли здесь делать конст-бросок? - PullRequest
2 голосов
/ 23 марта 2012

Я написал свою собственную реализацию универсального дерева, и когда я пишу для нее итераторы, у меня возникают проблемы с правильностью констант.Проблема, с которой я сталкиваюсь в настоящее время, заключается в следующем:

Это заголовочный файл для итератора DFS, который я написал:

template<class Item>
class DFSIterator
{
public:
    DFSIterator(const Item& rRootNode);
    ~DFSIterator();
    DFSIterator* First();
    DFSIterator* operator++(int rhs);
    Item* operator*() const;
    Item* operator->() const;
    bool isDone() const;

    template <class Node> friend class Node;

private:
    void initListIterator(const Item* currentNode);

    bool m_bIsDone;
    const Item* m_pRootNode;
    const Item* m_pCurrentNode;
    ListIterator<Item>* m_pCurrentListIter;
    std::map<const Item*, ListIterator<Item>*>  m_listMap;
};

Итак, меня беспокоит бит разыменования:

template<class Item>
Item* DFSIterator<Item>::operator*() const
{
    if(isDone())
    {
        return NULL;
    }
    else
    {
        return const_cast<Item*>(m_pCurrentNode);
    }
}

Уместно ли там делать const_cast?Мне интересно, если это вызовет проблемы, если пользователь поместит const объекты в контейнер?

Ответы [ 5 ]

2 голосов
/ 23 марта 2012

Не выбрасывайте const, так как он нарушит его значение! Есть причина, по которой в STL есть класс const_iterator и итератор. Если вам нужна постоянная корректность, вам нужно реализовать два отдельных класса итераторов. Одна из целей итератора - имитировать указатель сохранения. Таким образом, вы не хотите возвращать null, если вы находитесь за пределами диапазона итераций, вы хотите поднять отладочное утверждение, если это действительно произойдет.

Класс const iterator может выглядеть примерно так:

template<class Item>
class DFSIteratorConst
{
public:
    DFSIteratorConst(const Item& node)
    {
        m_pCurrentNode = &node;
    };

    const Item& operator*() const
    {
        assert(!IsDone());
        return *m_pCurrentNode;
    }

    void operator++()
    {
        assert(!IsDone());
        m_pCurrentNode = m_pCurrentNode->next;
    }

    operator bool() const
    {
        return !IsDone();
    }

    bool IsDone() const
    {
        return m_pCurrentNode == nullptr;
    }

private:
    Item const * m_pCurrentNode;
};

Несколько замечаний:

  • рассмотрите возможность возврата ссылки на узел (константная ссылка). Это приводит к тому, что вы не сможете вернуть ноль, если итератор находится за последним элементом. Разыменование прошлого итератора конца обычно является плохим поведением, и вы хотите найти эти проблемы во время отладочных сборок и тестирования
  • у класса const iterator есть указатель на элемент const. Это означает, что он может изменить указатель (иначе вы не сможете выполнить итерацию), но может не изменить сам элемент
  • неявное преобразование в bool целесообразно, если вы хотите проверить итератор в цикле while. Это позволяет вам снова написать: while(it) { it++; } вместо while(!it.IsDone()) { it++; }, что напоминает классический указатель.
  • используйте nullptr , если доступно

Как я вижу по вашему использованию std::map вы уже используете STL. Может быть, было бы проще использовать существующие итераторы STL?

2 голосов
/ 23 марта 2012

Поскольку ваш конструктор использует const Item, ваш оператор должен вернуть константный указатель.

Если вы хотите вернуть неконстантный элемент, вы должны использовать неконстантный параметр для вашего конструктора. Решение состоит в том, чтобы иметь базовый класс, использующий константные объекты, и подкласс, работающий с неконстантными объектами (немного по-другому, например, в NSString и NSMutableString).

1 голос
/ 23 марта 2012

Реальная проблема заключается в том, что этот код / ​​интерфейс «лжет».

Конструктор говорит: я не изменяю вам (корень).Но итератор раздает изменяемый элемент.

Чтобы быть «честным», либо конструктор получает изменяемый корень, даже если он не будет изменен в этом классе, либо этот класс не дает изменяемый элемент.

Однако, сам Item определяет, будет ли он отдавать изменчивых детей.В зависимости от того код может не скомпилироваться.Может быть, в этом причина вашей конструкции.

Длинный разговор, короткий смысл: этот дизайн плохой и его следует изменить

Возможно, вам нужны два шаблона в зависимости от "const" -ности заданногодети далеко

1 голос
/ 23 марта 2012

Обычно вы пишете две версии итераторов: iterator и const_iterator.

. Существуют приемы с шаблонами, позволяющие избежать дублирования кода, описанного в документации библиотеки Boost.Iterator.Посмотрите, как адаптер итератора определяется как довольно длинный.

Дело в том, что вы напишите BaseIterator, который const сознает, а затем предоставите псевдонимы:

  • typedef BaseIterator<Item> Iterator;
  • typedef BaseIterator<Item const> ConstIterator;

Хитрость заключается в том, как вы определяете конструктор преобразования так, чтобы от Iterator до ConstIterator было возможно, нообратного нет.

1 голос
/ 23 марта 2012

const_cast сам по себе всегда безопасен, но любая попытка записи в const_cast ed значений, которые были объявлены const, является неопределенным поведением.

В этом случае, если ваш возвращенный указатель указывает на значение, которое объявлено const, и вы пытаетесь изменить его, вы получите неопределенное поведение.


Правило большого пальца: Если вам нужно использовать const_cast для чего-либо, кроме взаимодействия с const-некорректным кодом, ваш дизайн нарушен.


Стандарт гласит: 7.1.6.1.cv-qualifiers :

За исключением того, что любой член класса, объявленный изменяемым (7.1.1), может быть изменен, любая попытка изменить объект const в течение его времени жизни (3.8) приводит к неопределенному поведению.

...