2

如果以下是好的或坏的设计,我会每天尝试学习新的东西。

我正在实现一个A将自身对象缓存在静态私有成员变量中的类std::map<> cache。的用户A应该只能访问指向地图中元素的指针,因为完整的副本A很昂贵且不需要。A仅当地图中尚不可用时才会创建新的,因为构建A需要一些繁重的工作。好的,这里有一些代码:

class B;

class A {
    public:
        static A* get_instance(const B & b, int x) {
            int hash = A::hash(b,x);
            map<int, A>::iterator found = cache.find(hash);
            if(found == cache.end())
                found = cache.insert(make_pair(hash, A(b,x))).first;
            return &(found->second);
        }
        static int hash(B & b, int x) { 
            // unique hash function for combination of b and x
        }
        // ...

    private:
        A(B & b, int x) : _b(b), _x(x) { 
            // do some heavy computation, store plenty of results 
            // in private members
        }
        static map<int, A> cache;
        B _b;
        int _x; // added, so A::hash() makes sense (instead of B::hash())
        // ...
 };

上面的代码有什么问题吗?是否有任何陷阱,我是否错过了内存管理问题或其他任何问题?

感谢您的反馈意见!

4

3 回答 3

3

该实现旨在仅允许您通过 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逻辑将确保它仅创建一次。

于 2011-03-01T15:44:19.597 回答
1

任何时候你使用全局变量(在这种情况下是静态映射),如果跨多个线程使用,你必须担心并发问题。例如,如果两个线程试图同时获取一个特定实例,它们都可能创建一个导致重复的对象。更糟糕的是,如果他们都尝试同时更新地图,地图可能会损坏。您必须使用互斥锁来控制对容器的访问。

如果它只是单线程的,那么在有人决定将来需要将其制成多线程之前就没有问题。

同样作为样式说明,虽然以下划线+小写字母开头的名称在技术上是合法的,但避免任何以下划线开头的符号将避免可能意外违反规则并出现奇怪的行为。

于 2011-03-01T15:43:55.590 回答
1

我认为这些是你在 A 中混合在一起的 3 个独立的东西:

  • A 类本身(它的实例应该做什么)。
  • 出于缓存目的池化实例
  • 对某种类型有这样一个静态单例池

我认为它们应该在代码中分开,而不是在 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*/
}  
于 2011-03-01T16:21:27.253 回答