stl map operator[] 不好？

Question

我的代码审阅者指出，map 的 operator[] 的使用非常糟糕，会导致错误：

map[i] = new someClass;    // potential dangling pointer when executed twice

或者

if (map[i]==NULL) ...      // implicitly create the entry i in the map 

虽然我在阅读了insert()更好的API后理解了风险，因为它检查重复，因此可以避免发生悬空指针，我不明白如果处理得当，为什么[]可以根本不用？

我选择地图作为我的内部容器正是因为我想使用它快速且不言自明的索引功能。

我希望有人可以和我多争论或者站在我这边:)

Answer 1

如果你的地图是这样的：

std::map< int, int* >

然后你输了，因为下一个代码 sn-p 会泄漏内存：

std::map< int, int* > m;
m[3] = new int( 5 );
m[3] = new int( 2 );

---

如果处理得当，为什么[]根本不能用？

如果您正确地测试了您的代码，那么您的代码应该仍然无法通过代码审查，因为您使用了原始指针。

除此之外，如果使用得当，使用map::operator[] 没有任何问题。但是，使用插入/查找方法可能会更好，因为可能会进行静默地图修改。

Answer 2

1. operator [] 被避免插入，因为同样的原因 你在你的问题中提到。它不检查重复键 并覆盖现有的。
2. operator [] 在std::map 中的搜索大多避免使用。 因为，如果您的map 中不存在密钥，那么operator [] 会静默创建新密钥并初始化它（通常是 0)。这可能不是在所有情况下都是可取的。一个应该使用 [] 仅在需要创建密钥时，如果它不存在。

Answer 3

你说得对，map::operator[] 必须小心使用，但它可能非常有用：如果你想在地图中找到一个元素，如果没有，则创建它：

someClass *&obj = map[x];
if (!obj)
    obj = new someClass;
obj->doThings();

地图中只有一个查找。 如果new 失败，您当然可以从映射中删除 NULL 指针：

someClass *&obj = map[x];
if (!obj)
    try
    {
        obj = new someClass;
    }
    catch (...)
    {
        obj.erase(x);
        throw;
    }
obj->doThings();

当然，如果你想找到一些东西，而不是插入它：

std::map<int, someClass*>::iterator it = map.find(x); //or ::const_iterator
if (it != map.end())
{
    someClass *obj = it->second;
    obj->doThings();
}

Answer 4

诸如“使用地图的 operator[] 非常糟糕”之类的声明应该始终是几乎宗教信仰的警告信号。但与大多数此类说法一样，某处潜伏着一些真相。然而，这里的事实与 C++ 标准库中的几乎所有其他构造一样：小心并知道自己在做什么。您可以（意外地）滥用几乎所有内容。

一个常见问题是潜在的内存泄漏（假设您的地图拥有这些对象）：

std::map<int,T*> m;
m[3] = new T;
...
m[3] = new T;

这显然会泄漏内存，因为它会覆盖指针。在这里正确使用 insert 也不是一件容易的事，而且很多人犯了一个无论如何都会泄漏的错误，比如：

std::map<int,T*> m;
minsert(std::make_pair(3,new T));
...
m.insert(std::make_pair(3,new T));

虽然这不会覆盖旧指针，但它不会插入新指针，也会泄漏它。插入的正确方法是（可能使用智能指针更好地增强）：

std::map<int,T*> m;
m.insert(std::make_pair(3,new T));
....
T* tmp = new T;
if( !m.insert(std::make_pair(3,tmp)) )
{
    delete tmp;
}

但这也有点难看。我个人更喜欢这种简单的情况：

std::map<int,T*> m;

T*& tp = m[3];
if( !tp )
{
    tp = new T;
}

但这可能与您的代码审查员不允许使用 op[] 的个人偏好相同...

Answer 5

operator[] 唯一有用的时候（我能想到的）是当你想设置一个键的值（如果它已经有值就覆盖它），并且你知道它是 覆盖安全（应该是这样，因为您应该使用智能指针，而不是原始指针）并且 默认构造便宜，并且在某些情况下，该值应该具有 无抛出构造和分配。

例如（类似于你的第一个例子）

std::map<int, std::unique_ptr<int>> m;
m[3] = std::unique_ptr<int>(new int(5));
m[3] = std::unique_ptr<int>(new int(3)); // No, it should be 3.

否则，根据上下文有几种方法可以做到这一点，但我建议始终使用通用解决方案（这样你就不会弄错）。

找到一个值，如果它不存在就创建它：

1.通用解决方案（推荐，因为它始终有效）

std::map<int, std::unique_ptr<int>> m;
auto it = m.lower_bound(3);
if(it == std::end(m) || m.key_comp()(3, it->first))
   it = m.insert(it, std::make_pair(3, std::unique_ptr<int>(new int(3)));

2。具有廉价的默认价值构造

std::map<int, std::unique_ptr<int>> m;
auto& obj = m[3]; // value is default constructed if it doesn't exists.
if(!obj)
{
   try
   {
      obj = std::unique_ptr<int>(new int(3)); // default constructed value is overwritten.
   }
   catch(...)
   {
      m.erase(3);
      throw;
   }
}

3.具有廉价的默认构造和不抛出值的插入

std::map<int, my_objecct> m;
auto& obj = m[3]; // value is default constructed if it doesn't exists.
if(!obj)
   obj = my_objecct(3);

注意：您可以轻松地将通用解决方案包装到辅助方法中：

template<typename T, typename F>
typename T::iterator find_or_create(T& m, const typename T::key_type& key, const F& factory)
{
    auto it = m.lower_bound(key);
    if(it == std::end(m) || m.key_comp()(key, it->first))
       it = m.insert(it, std::make_pair(key, factory()));
    return it;
}

int main()
{
   std::map<int, std::unique_ptr<int>> m;
   auto it = find_or_create(m, 3, []
   {
        return std::unique_ptr<int>(new int(3));
   });
   return 0;
}

请注意，我为创建案例传递了一个模板化工厂方法而不是一个值，这样在找到值并且不需要创建值时就没有开销。由于 lambda 作为模板参数传递，编译器可以选择内联它。

Answer 6

 map[i] = new someClass;    // potential dangling pointer when executed twice

这里，问题不在于地图的operator[]，而是*缺少智能指针。 您的指针应该存储到某个 RAII 对象（例如智能指针）中，该对象会立即获得分配对象的所有权，并确保它会被释放。

如果您的代码审阅者忽略了这一点，而是说您应该热衷于 operator[]，请给他们买一本好的 C++ 教科书。

if (map[i]==NULL) ...      // implicitly create the entry i in the map 

确实如此。但那是因为operator[] 被设计成不同的行为。显然，你不应该在它做错事的情况下使用它。

Answer 7

通常的问题是 operator[] 隐式创建一个与传入的键关联的值，如果该键尚未出现，则在映射中插入一个新对。从那时起，这可能会破坏您的逻辑，例如当你搜索某个键是否存在时。

map<int, int> m;
if (m[4] != 0) {
  cout << "The object exists" << endl; //furthermore this is not even correct 0 is totally valid value
} else {
  cout << "The object does not exist" << endl;
}
if (m.find(4) != m.end()) {
  cout << "The object exists" << endl; // We always happen to be in this case because m[4] creates the element
}

我建议仅当您知道您将引用地图中已经存在的键时才使用 operator[]（顺便说一句，这种情况并不常见）。

Answer 8

[] 根本不是问题。在容器中存储原始指针是个问题。

Answer 9

地图的operator[] 本身并没有错，只要它 语义对应于你想要的。问题是定义你的 想要（并且知道operator[] 的确切语义）。有的时候 当隐式创建具有默认值的新条目时 不存在正是您想要的（例如，计算文本中的单词 文档，其中++ countMap[word] 就是您所需要的）；有 很多时候它不是。

您的代码中更严重的问题可能是您正在存储指针 在地图中。更自然的解决方案可能是使用map <keyType, someClass>，而不是map <keyType, SomeClass*>。但同样，这 取决于所需的语义；比如我用了很多map 在程序启动时初始化一次，并带有指向静态的指针 实例。如果您是 map[i] = ... 处于初始化循环中， 在启动时执行一次，可能没有问题。如果是什么 在代码的许多不同地方执行，可能有一个 问题。

解决问题的方法不是禁止operator[]（或映射到 指针）。解决方案是从指定确切的语义开始 你需要。如果std::map 不直接提供它们（它很少 确实），编写一个小的包装类，它定义了你的确切语义 想要，使用std::map 来实现它们。因此，你的包装器 operator[] 可能是：

MappedType MyMap::operator[]( KeyType const& key ) const
{
    MyMap::Impl::const_iterator elem = myImpl.find( key );
    if ( elem == myImpl.end() )
        throw EntryNotFoundError();
    return elem->second;
}

或：

MappedType* MyMap::operator[]( KeyType const& key ) const
{
    MyMap::Impl::const_iterator elem = myImpl.find( key );
    return elem == myImpl.end()
        ?   NULL   //  or the address of some default value
        :   &elem->second;
}

同样，您可能希望使用 insert 而不是 operator[] 你真的想插入一个不存在的值。

而且我几乎从未见过你会立即插入 newed 对象到地图中。使用new 的常见原因和 delete 是有问题的对象有一些特定的生命周期 他们自己的（并且不可复制——尽管不是绝对规则，如果 你是newing 一个支持复制和赋值的对象，你是 可能做错了什么）。当映射类型是指针时， 那么要么指向的对象是静态的（并且地图更多或 初始化后的常数较小），或者插入和删除是 在类的构造函数和析构函数中完成。 （但这只是 一般规则；当然也有例外。）