这是对私有构造函数的好用吗？

Question

每天都在尝试学习新的东西，如果以下是好的或坏的设计，我会很感兴趣。

我正在实现一个类A，它将自身的对象缓存在一个静态私有成员变量std::map<> cache 中。 A 的用户应该只能访问指向映射中元素的指针，因为 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())
        // ...
 };

上面的代码有什么问题吗？有没有什么坑， 我想念内存管理问题还是其他什么？

感谢您的反馈！

Answer 1

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

Answer 2

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

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

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

Answer 3

我认为这些是你在 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*/
}