Этот контейнер указателя C ++ безопасен? - PullRequest
Новая
       2

Этот контейнер указателя C ++ безопасен?

1 голос
/ 06 ноября 2010

Мне нужен безопасный контейнер указателя C ++, похожий на scoped_ptr boost, но с семантикой копирования, подобной значению. Я намерен использовать это для очень редко используемого элемента очень интенсивно используемого класса в самом внутреннем цикле приложения, чтобы получить лучшую локальность памяти. Другими словами, мне не важна производительность этого класса, если его «встроенная» нагрузка на память мала.

Я начал со следующего, но я не настолько искусен в этом; следующий безопасно? Я заново изобретаю колесо, и если да, то где мне искать? 

template <typename T> 
class copy_ptr {
    T* item;
public:
    explicit copy_ptr() : item(0) {}
    explicit copy_ptr(T const& existingItem) : item(new T(existingItem)) {}
    copy_ptr(copy_ptr<T> const & other) : item(new T(*other.item)) {}
    ~copy_ptr()  { delete item;item=0;}

    T  * get() const {return item;}
    T & operator*() const {return *item;}
    T * operator->() const {return item;}
};

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

Ответы [ 3 ]

5 голосов
/ 06 ноября 2010

Нет, это не так.

Вы забыли оператор присваивания.

Вы можете либо запретить назначение (странно, когда копирование разрешено), объявив оператор присваивания закрытым (а нереализации), или вы можете реализовать его следующим образом: 

copy_ptr& operator=(copy_ptr const& rhs)
{
  using std::swap;

  copy_ptr tmp(rhs);
  swap(this->item, tmp.item);
  return *this;
}

Вы также забыли в конструкторе копирования, что other.item может быть нулевым (как следствие конструктора по умолчанию), выберите свой вариант:

// 1. Remove the default constructor

// 2. Implement the default constructor as
copy_ptr(): item(new T()) {}

// 3. Implement the copy constructor as
copy_ptr(copy_ptr const& rhs): item(other.item ? new T(*other.item) : 0) {}

Для поведения, подобного значению, я бы предпочел 2, поскольку значение не может быть нулевым.Если вы хотите разрешить нулевое значение, введите assert(item); в operator-> и operator*, чтобы обеспечить корректность (в режиме отладки), или сгенерируйте исключение (как хотите).

Наконец, item = 0 вдеструктор бесполезен: вы не можете использовать объект, если он все равно был уничтожен, не вызывая неопределенного поведения ...

Есть также замечание Роджера Пейта о распространении константности, которое должно быть более "ценностным", но это большедело семантики, чем правильности.

3 голосов
/ 06 ноября 2010

Вы должны "передать" константу типа copy_ptr: 

T* get() { return item; }
T& operator*() { return *item; }
T* operator->() { return item; }

T const* get() const { return item; }
T const& operator*() const { return *item; }
T const* operator->() const { return item; }

Указание T не требуется в копии ctor: 

copy_ptr(copy_ptr const &other) : item (new T(*other)) {}

Почему вы сделали ctor по умолчанию явным? Обнуление указателя в dtor имеет смысл, только если вы планируете где-нибудь UB ...

Но это все мелкие проблемы, то, что у вас есть, в значительной степени так. И да, я видел это изобретенное много раз, но люди, как правило, слегка подправляют семантику каждый раз. Вы можете взглянуть на boost :: option, поскольку это почти то, что вы написали здесь, когда представляете его, если только вы не добавляете семантику перемещения и другие операции.

1 голос
/ 06 ноября 2010

В дополнение к тому, что сказал Роджер, вы можете Google 'clone_ptr' для идей / сравнения.

...