insertions into c++ map STL container is failing

Please see the following code to understand my question better.

class compareByValue {                                                                             
  public:                                                                                          
    bool operator()(const string* s1, const string* s2) const                                      
    {                                                                                              
      if (s1 == s2)                                                                                
        return true;                                                                               
      if ((s1==NULL) || (s2==NULL))                                                                
        return false;                                                                              
      return (0 == s1->compare(s2->c_str()));                                                      
    }         开发者_如何学C                                                                                                                                                                                    
}; 

map<string*, string*, compareByValue> nodeIdToIpAddress;

for (int i = 0; i < nrec; ++i) {
  nodeIdToIpAddress[ptr1[i]] = ptr2[i];                                              
  cout << "Added " << *(ptr1[i]) << " , " << *(ptr2[i]) << endl;
}

cout << "map has " << nodeIdToIpAddress.size() << " elements!" << endl;

I have a map that maintains key,value pairs which are pointers to string objects. I'm sure that neither keys nor values are NULL pointers. When I run the above program (well, I skipped surrounding code to make it easier to understand), "Added ... ..." gets printed 49 times. ptr1[i], ptr2[i] are pointers to string objects and are not NULL pointers because my program doesn't segfault.

The problem I have is, when I print size of map at the end, it says map has only 1 element in it.

I would appreciate if someone can give me directions to find the fix. Thanks in advance.

EDIT: @Mark solution worked like charm for me. thanks

EDIT2: After seeing valuable feedback from @Mark and @James, I think I don't need to store pointers to strings in my map. I'm going to change my code to store strings as key/values which means I don't need custom comparator functor. Thanks a lot.

Your comparator is wrong: if (s1 == s2) must return false and the comparator must produce a strict weak ordering.

[For what it's worth, using a pointer type as the key for a std::map is unusual at best.]

Your comparison operator isn't right. As long as you can assume that no nulls are added, you can throw if you detect any:

bool operator()(const string* s1, const string* s2) const                                      
{                                                                                              
  if (!s1 || !s2) throw std::runtime_error("Whatever");

  return *s1 < *s2;
}                                                                                 

EDIT: I should have elaborated on the reasoning here a bit more. The comparison operation that you pass into a std::map (or set or sort for example) must satisfy strict weak ordering. This basically means that x OP x is false, if x OP y is true then y OP x is false, and that x OP y, y OP z implies x OP z (as well as other rules regarding items that can't be compared).

Your function didn't satisfy this in several ways: if (s1 == s2) needs to return false. if ((s1==NULL) || (s2==NULL)) can't return false because it would imply that null < anything and anything < null. You'd have to split it up and return true for s1 null and false for s2 null. return (0 == s1->compare(s2->c_str())); is incorrect because it's a three-way result rather than a < relation.

Since you specified that nulls would never be inserted, the simplest resolution seemed to be to throw an exception in that case (if either was null) and thenuse the string built-in operator.

All that being said now, I would strongly consider NOT storing strings by pointer in containers unless you have actual evidence that it's a performance bottleneck. A simpler design is easier to maintain and keep correct, which tends to lead to better programs long-term.