`edgeHash` is not safe to use for directed cyclic graphs

[nrkramer]

`edgeHash` is not safe to use for directed cyclic graphs

[sbaldu]

I agree that in your example the hash index given by this function will be the same, so there is a conflict in the table, but the equality operator should be able to distinguish between the two edges.
In the PointerHash.hpp file where I defined the hash function, I also added an overload of the equality operator for shared pointers of edges:

template <typename T>
bool operator==(shared<const Edge<T>> p1, shared<const Edge<T>> p2) {
  return p1->getNodePair().first->getUserId() ==
             p2->getNodePair().first->getUserId() &&
         p2->getNodePair().second->getUserId() ==
             p2->getNodePair().second->getUserId();
}

In this way, even if two edges are given the same hash index, they won't be seen as the same, and will both be added to the set, because if they create a cycle the order of the nodes in the nodepair will be swapped.

Does it make sense or am I missing something?

[ZigRazor]

I think this can be verified with a simple testcase. Anyone can elaborate this testcase with failure result?

[sbaldu]

@ZigRazor I'll do it

[nrkramer]

but the equality operator should be able to distinguish between the two edges.

I think I missed this. This does alleviate my concern, thanks! I'm still seeing some odd behavior where unique DirectedEdge's are not being added to the container, so I'll continue to investigate.

I'll open a PR shortly with a test to demonstrate my issue (realistically, I should have started with that).