Это хорошее использование частного конструктора? - PullRequest
Новая
       27

Это хорошее использование частного конструктора?

2 голосов
/ 01 марта 2011

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

Я реализую класс A, который кэширует свои объекты в статической закрытой переменной-члене std::map<> cache. Пользователь A должен иметь доступ только к указателям на элементы на карте, потому что полная копия A стоит дорого и не нужна. Новый A создается только в том случае, если он еще не доступен на карте, поскольку для построения A требуется некоторая тяжелая работа. Хорошо, вот код:

* * 1010

Что-то не так с кодом выше? Есть ли подводные камни, я пропускаю проблемы управления памятью или что-нибудь еще?

Спасибо за ваш отзыв!

Ответы [ 3 ]

3 голосов
/ 01 марта 2011

Реализация предназначена только для того, чтобы позволить вам создавать элементы с помощью get_instance (). В идеале вы должны сделать свой конструктор копирования и оператор присваивания приватным.

Это не будет потокобезопасным. Вместо этого вы можете использовать следующее: 

const boost::once_flag BOOST_ONCE_INIT_CONST = BOOST_ONCE_INIT;

struct AControl
{
  boost::once_flag onceFlag;
  shared_ptr<A> aInst;

  void create( const B&b, int x )
  {
      aInst.reset( new A(b, x) );
  }

  AControl() : onceFlag( BOOST_ONCE_INIT_CONST )
  {
  }

  A& get( const B&b, int x )
  {
     boost::call_once( onceFlag, bind( &AOnceControl::create, this, b, x ) );
     return *aInst;
  }
};

Изменить карту на карта

Иметь мьютекс и использовать его таким образом: 

AControl * ctrl;
{
  mutex::scoped_lock lock(mtx);
  ctrl = &cache[hash];
}
return ctrl->get(b,x);

В идеале только get_instance () будет статичным в вашем классе. Все остальное является частной реализацией и входит в модуль компиляции вашего класса, включая AControl.

Обратите внимание, что вы могли бы сделать это намного проще, просто заблокировав весь процесс поиска на карте и создания, но затем вы блокируете дольше, пока выполняете длительный процесс построения. Так как это реализует блокировку на уровне записи после того, как вы вставили элемент. Более поздний поток может найти элемент неинициализированным, но логика boost::once обеспечит его создание ровно один раз.

1 голос
/ 01 марта 2011

Я думаю, что это 3 отдельные вещи, которые вы смешиваете внутри A:

  • сам класс А (что должны делать его экземпляры).
  • пул экземпляров для целей кэширования
  • с таким статическим пулом синглтона для определенного типа

Я думаю, что они должны быть отдельными в коде, а не все вместе внутри А.

Это значит:

  • напишите свой класс A без учета того, как он должен быть распределен.

  • написать универсальный модуль для выполнения кеша пула объектов в соответствии с:

*

template< typename T > class PoolHashKey { ... };

template< typename T > class PoolCache  
{  
//data  
  private: std::map< .... > map_;  

//methods  
    public: template< typename B > PoolKey< T > get_instance( B b );  
    public: void release_instance( PoolKey< T > );  
    // notice that these aren't static function members  
};
  • создайте одноэлементный экземпляр PoolCache и используйте его:

*

PoolCache<A>& myAPool()  
{  
    static PoolCache<A> s;  
    return s;  
    //you should use some safe singleton idiom.  
}  

int main()  
{  
  B b;  
  PoolKey<A> const aKey( myAPool().get_instance( b );  
  A* const a( aKey.get() );  
  //...  
  myAPool().release_instance( aKey ); //not using it anymore
  /*or else the destructor of PoolKey<A> should probably do some reference count and let the pool know this instace isn't needed anymore*/
}
1 голос
/ 01 марта 2011

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

Если он только однопоточный, тогда нет никаких проблем, пока кто-нибудь не решит, что в будущем его нужно сделать многопоточным.

Также в качестве примечания к стилю, хотя имена, начинающиеся с подчеркивания + строчной буквы, технически допустимы, избегайте любых символов, начинающихся с подчеркивания, что позволит избежать случайного нарушения правил и получения странного поведения.

...