C++ 感知复制插入到 std::map答案

【问题标题】：C++ Aware duplication insert into a std::mapC++ 感知复制插入到 std::map
【发布时间】：2012-10-01 18:21:54
【问题描述】：

我有一个关于在 C++ 中向 std::map 插入内容的问题。

到目前为止，这就是我的代码：

stringutils.hh:

...

  unsigned long hashSDBM(char *strToHash){
      unsigned char* str = new unsigned char[strlen(strToHash) + 1];
      strncpy( (char *) str, strToHash, strlen(strToHash) );

      unsigned long hash = 0;
      int c;

      while ((c = *str++)){
          hash = c + (hash <<6) + (hash <<16) - hash;
      }

      return hash;
  }

...

hashmap.hh

#include "stringutils.hh"

namespace{

using namespace std;

class MapElement{

    private:
        char* filename;
        char* path;

    public:
        MapElement(char* f, char* p):filename(f), path(p){}
        ~MapElement(){
           delete [] filename;
           delete [] path;
        }
        char* getFileName(){ return filename; }
        char* getPath(){ return path; }

};


class HashMap{

    private:
        map<long*, MapElement*> *hm;

        long hash(char* key);

    public:
        HashMap(){
           hm = new map<long*, MapElement*>();
        }
        ~HashMap(){
           delete hm;
        }
        long put(char* k, MapElement *v);
};

long HashMap::hash(char* key){
  return stringutils::hashSDBM(key);
}


long HashMap::put(char* k, MapElement *v){
  long *key = new long();
  *key = hash(k);
  pair<map<long*,MapElement*>::iterator, bool> ret;
  ret = hm->insert(std::pair<long*, MapElement*>(key, v));

  if(ret.second == false){
    cerr<<"Already exists: "<<ret.first->second->getFileName()<<endl;
    return *key;
  }
  cerr<<"INSERTED "<<*key<<endl;
  return 0;
}

main.cc:

HashMap *hm = new HashMap();


int main(void){

  MapElement *m1; 

  char a[] = "hello";
  char b[] = "world";
  m1 = new MapElement(a,b);
  hm->put(a, m1);

  char c[] = "thats";
  char d[] = "a test";
  m1 = new MapElement(c,d);
  hm->put(c, m1);

  char e[] = "hello";
  char f[] = "test";
  m1 = new MapElement(e,f);
  hm->put(e, m1);

  return 0;
}

它编译时没有任何错误或警告，当我启动它时，会生成以下输出：

插入 7416051667693574450

插入 8269306963433084652

插入 7416051667693574450

为什么第二次插入“hello”键没有任何效果？

【问题讨论】：

你应该解释你的代码是做什么的，而不是仅仅粘贴它。
您的代码（函数hashSDBM）内存泄漏。您应该使用 std::string 而不是原始的 char * 字符串。
这段代码存在严重的资源管理问题。 HashMap 析构函数不会释放为所有键和值分配的内存。 MapElement 析构函数永远不会被调用，但如果调用它，它会出现段错误，因为它会在指向自动变量的指针上盲目调用 delete[]，而不是动态分配内存。两个类都不能正确处理复制或分配。请停止使用pointers 和new。

标签： c++ hash insert duplicates stdmap

【解决方案1】：

std::map 中的键是唯一的。如果要允许重复键，请使用std::multimap。您正在使用的 map::insert 返回一对迭代器和一个bool。 bool 表示插入是否已实际插入（不是密钥是否已经存在）。

【讨论】：

【解决方案2】：

为什么第二次插入key没有任何作用？

您的键是一个指针，两个指向不同long 对象的具有相同值的指针是不同的键。不要过度使用指针，你真的会帮助自己。 C++ 不是 Java。

【讨论】：

MAP-doc 如果 ret->second == false，则密钥已经存在！我不想要多个键，但我想识别一个键是否已经存在！
@Hymir 忽略我之前的回答。

【解决方案3】：

天哪...在继续之前请阅读一本好 C++ 书籍，C++ 标签说明中有推荐的好书籍。

所以，这里的问题是您的代码使用指针......无处不在......并且指针的行为不像您认为的那样。许多语言（如 Java）具有普遍的引用类型：一切都只是一个引用。 C++ 不是这样的语言，它在一方面的指针/引用和另一方面的值之间有很大的不同。

在您的具体情况下，long* 是指向 long 的指针。就map 而言，两个不同的指针就是：distinct，无论它们指向什么值。

所以...我们需要摆脱这些指针。到处。并且停止在 C++ 中使用 C 习语。

stringutils.hh

  unsigned long hashSDBM(std::string const& strToHash){
      unsigned long hash = 0;

      for (char c: strToHash) {
          hash = c + (hash <<6) + (hash <<16) - hash;
      }

      return hash;
  }

简而言之：

不要在 C++ 中使用原始 char*，内存所有权不明确会导致泄漏/悬空指针
适当地使用const，不修改其参数的函数应采用const对其的引用
将 C++11 用于样式循环，它们与手动代码一样高效，同时更易于阅读且更难搞砸

hashmap.hh

namespace HashMap {

class MapElement{
public:
    MapElement(std::string f, std::string p):
        filename(f), path(p) {}

    std::string const& getFileName() const { return filename; }
    std::string const& getPath() const { return path; }

private:
    std::string filename;
    std::string path;
};

让我们从这里开始：

标头中没有匿名命名空间，它不会像您认为的那样做（阅读它们）
没有原始指针
在商务舱中不要摆弄资源
常量正确性很重要
先介绍公共 API，这是用户感兴趣的内容

向前：

class HashMap{
public:
    unsigned long put(std::string const& k, MapElement v);

private:
    static unsigned long hash(std::string const& key);

    std::map<unsigned long, MapElement> hm;
};

inline unsigned long HashMap::hash(std::string const& key){
    return stringutils::hashSDBM(key);
}

inline unsigned long HashMap::put(std::string const& k, MapElement v){
    unsigned long const key = hash(k);

    auto const ret = hm.emplace(key, v);

    if (ret.second == false){
        std:: cerr << "Already exists: " << ret.first->second.getFileName() << "\n";
        return key;
    }
    std::cerr << "INSERTED " << key << "\n";
    return 0;
}

好吧……

不需要那么多指针，没有它们代码更简单！
内部hash函数不访问任何状态，将其设为static
尽可能在最后一刻声明变量，并立即初始化它们...它让您可以使用auto，而不是显式命名过于复杂的类型
std::endl 并没有像你想象的那样做（提示：它会刷新缓冲区！最慢的 I/O 操作！），只需使用普通的 "\n" 代替

补充说明：

如果密钥必须是文件名，为什么还要让用户提交密钥？您可以改为从MapElement 对象中读取它...或在发生冲突时打印key（而不是文件名），以防它们不同
哈希不是唯一的，如果两个不同的文件名哈希到相同的数字，您将拒绝第二个...您应该使用复合键（哈希 + 文件名）
插入时返回0，不插入时返回key...但没有什么能阻止密钥成为0，因此收到0 会让用户怀疑发生了什么

main.cpp

int main(void){
    HashMap::HashMap hm;

    hm.put("hello", MapElement("hello", "world"));
    hm.put("thats", MapElement("thats", "a test"));
    hm.put("hello", MapElement("hello", "test"));

    return 0;
}

最后：

避免使用全局变量
无需命名所有临时对象

【讨论】：