From d6a2ab3e8c33d530b9b0833291aad3f3175da2e0 Mon Sep 17 00:00:00 2001
From: David Rosca <roscadav@fit.cvut.cz>
Date: Sun, 22 Mar 2015 11:58:26 +0100
Subject: [PATCH] UndirectedEdge: Fix compare and hash for Edge(n1, n2) -
 Edge(n2, n1)

---
 .../src/graph/undirected/UndirectedEdge.cpp   | 27 ++++++-
 .../src/graph/undirected/UndirectedEdge.h     |  1 +
 .../test-src/graph/GraphElementsTest.cpp      | 81 ++++++++++++++++---
 alib2data/test-src/graph/GraphElementsTest.h  |  4 +
 alib2data/test-src/graph/GraphTest.cpp        |  6 ++
 5 files changed, 105 insertions(+), 14 deletions(-)

diff --git a/alib2data/src/graph/undirected/UndirectedEdge.cpp b/alib2data/src/graph/undirected/UndirectedEdge.cpp
index fed6072e48..79327f8dd2 100644
--- a/alib2data/src/graph/undirected/UndirectedEdge.cpp
+++ b/alib2data/src/graph/undirected/UndirectedEdge.cpp
@@ -138,14 +138,17 @@ const label::Label &UndirectedEdge::getName() const
 
 int UndirectedEdge::compare(const UndirectedEdge &other) const
 {
+	auto a = sortedNodes();
+	auto b = other.sortedNodes();
+
 	int res = name.getData().compare(other.name.getData());
 	if (res == 0) {
-		if (first.compare(other.second) == 0 && second.compare(other.first) == 0) {
+		if (a.first.compare(b.second) == 0 && a.second.compare(b.first) == 0) {
 			return 0;
 		}
-		res = first.compare(other.first);
+		res = a.first.compare(b.first);
 		if (res == 0) {
-			res = second.compare(other.second);
+			res = a.second.compare(b.second);
 		}
 	}
 	return res;
@@ -171,7 +174,23 @@ std::ostream &operator<<(std::ostream &out, const UndirectedEdge &node)
 
 void UndirectedEdge::dump(std::ostream &os) const
 {
-	os << "(UndirectedEdge " << first << " -> " << second << ")";
+	auto nodes = sortedNodes();
+	os << "(UndirectedEdge " << nodes.first << " -> " << nodes.second << ")";
+}
+
+std::pair<Node, Node> UndirectedEdge::sortedNodes() const
+{
+	std::pair<Node, Node> out;
+
+	if (first < second) {
+		out.first = first;
+		out.second = second;
+	} else {
+		out.first = second;
+		out.second = first;
+	}
+
+	return out;
 }
 
 } // namespace graph
diff --git a/alib2data/src/graph/undirected/UndirectedEdge.h b/alib2data/src/graph/undirected/UndirectedEdge.h
index 2991d83571..379300e591 100644
--- a/alib2data/src/graph/undirected/UndirectedEdge.h
+++ b/alib2data/src/graph/undirected/UndirectedEdge.h
@@ -54,6 +54,7 @@ public:
 
 private:
 	void dump(std::ostream &os) const;
+	std::pair<Node, Node> sortedNodes() const;
 
 	Node first;
 	Node second;
diff --git a/alib2data/test-src/graph/GraphElementsTest.cpp b/alib2data/test-src/graph/GraphElementsTest.cpp
index 721d60c7b8..0910803002 100644
--- a/alib2data/test-src/graph/GraphElementsTest.cpp
+++ b/alib2data/test-src/graph/GraphElementsTest.cpp
@@ -89,11 +89,12 @@ void GraphElementsTest::testEqual()
 	CPPUNIT_ASSERT(n2 != n1);
 	CPPUNIT_ASSERT(n3 != n1);
 
-	CPPUNIT_ASSERT_EQUAL(0, std::compare<graph::Node>()(n1, n1_));
-	CPPUNIT_ASSERT_EQUAL(0, std::compare<graph::Node>()(n1, n1));
-	CPPUNIT_ASSERT(std::compare<graph::Node>()(n1, n2) != 0);
-	CPPUNIT_ASSERT(std::compare<graph::Node>()(n1, n3) != 0);
-	CPPUNIT_ASSERT(std::compare<graph::Node>()(n1, n3_) != 0);
+	std::compare<graph::Node> n_cmp;
+	CPPUNIT_ASSERT_EQUAL(0, n_cmp(n1, n1_));
+	CPPUNIT_ASSERT_EQUAL(0, n_cmp(n1, n1));
+	CPPUNIT_ASSERT(n_cmp(n1, n2) != 0);
+	CPPUNIT_ASSERT(n_cmp(n1, n3) != 0);
+	CPPUNIT_ASSERT(n_cmp(n1, n3_) != 0);
 
 	// DirectedEdge
 	graph::DirectedEdge de1(n1, n2);
@@ -115,11 +116,12 @@ void GraphElementsTest::testEqual()
 	CPPUNIT_ASSERT(de2 != de1);
 	CPPUNIT_ASSERT(de3 != de1);
 
-	CPPUNIT_ASSERT_EQUAL(0, std::compare<graph::DirectedEdge>()(de1, de1_));
-	CPPUNIT_ASSERT_EQUAL(0, std::compare<graph::DirectedEdge>()(de1, de1));
-	CPPUNIT_ASSERT(std::compare<graph::DirectedEdge>()(de1, de2) != 0);
-	CPPUNIT_ASSERT(std::compare<graph::DirectedEdge>()(de1, de3) != 0);
-	CPPUNIT_ASSERT(std::compare<graph::DirectedEdge>()(de1, de3_) != 0);
+	std::compare<graph::DirectedEdge> d_cmp;
+	CPPUNIT_ASSERT_EQUAL(0, d_cmp(de1, de1_));
+	CPPUNIT_ASSERT_EQUAL(0, d_cmp(de1, de1));
+	CPPUNIT_ASSERT(d_cmp(de1, de2) != 0);
+	CPPUNIT_ASSERT(d_cmp(de1, de3) != 0);
+	CPPUNIT_ASSERT(d_cmp(de1, de3_) != 0);
 
 	// UndirectedEdge
 	graph::UndirectedEdge ue1(n1, n2);
@@ -143,5 +145,64 @@ void GraphElementsTest::testEqual()
 	CPPUNIT_ASSERT(ue1 != ue3);
 	CPPUNIT_ASSERT(ue2 != ue1);
 	CPPUNIT_ASSERT(ue3 != ue1);
+
+	std::compare<graph::UndirectedEdge> u_cmp;
+	CPPUNIT_ASSERT_EQUAL(0, u_cmp(ue1, ue1_));
+	CPPUNIT_ASSERT_EQUAL(0, u_cmp(ue1, ue1));
+	CPPUNIT_ASSERT_EQUAL(0, u_cmp(ue1, ue1_s));
+	CPPUNIT_ASSERT_EQUAL(0, u_cmp(ue1_, ue1_s));
+	CPPUNIT_ASSERT(u_cmp(ue1, ue2) != 0);
+	CPPUNIT_ASSERT(u_cmp(ue1, ue3) != 0);
+	CPPUNIT_ASSERT(u_cmp(ue1, ue3_) != 0);
+}
+
+// testLessThan and testHash tests the UndirectedEdge for
+// the case when UndirectedEdge(n1, n2) == UndirectedEdge(n2, n1)
+// and thus they must compare exactly the same against other edges
+// and also have the same hash
+void GraphElementsTest::testLessThan()
+{
+	// Common
+	graph::Node n1("n1");
+	graph::Node n2("n2");
+	graph::Node n3("n3");
+	graph::Node n4("n4");
+	graph::Node n5("n5");
+
+	// UndirectedEdge
+	graph::UndirectedEdge ue1(n1, n2);
+	graph::UndirectedEdge ue1_(n2, n1);
+	graph::UndirectedEdge ue2(n1, n3);
+	graph::UndirectedEdge ue3(n1, n3);
+	graph::UndirectedEdge ue4(n1, n4);
+	graph::UndirectedEdge ue5(n5, n1);
+
+	CPPUNIT_ASSERT_EQUAL(ue1 < ue1_, ue1_ < ue1_);
+	CPPUNIT_ASSERT_EQUAL(ue1 < ue2, ue1_ < ue2);
+	CPPUNIT_ASSERT_EQUAL(ue1 < ue3, ue1_ < ue3);
+	CPPUNIT_ASSERT_EQUAL(ue1 < ue4, ue1_ < ue4);
+	CPPUNIT_ASSERT_EQUAL(ue1 < ue5, ue1_ < ue5);
+}
+
+void GraphElementsTest::testHash()
+{
+	// Common
+	graph::Node n1("n1");
+	graph::Node n2("n2");
+	graph::Node n3("n3");
+	graph::Node n4("n4");
+
+	// UndirectedEdge
+	graph::UndirectedEdge ue1(n1, n2);
+	graph::UndirectedEdge ue1_(n2, n1);
+	graph::UndirectedEdge ue2(n1, n3);
+	graph::UndirectedEdge ue2_(n3, n1);
+	graph::UndirectedEdge ue3(n2, n4);
+	graph::UndirectedEdge ue3_(n4, n2);
+
+	std::hash<graph::UndirectedEdge> hash_func;
+	CPPUNIT_ASSERT_EQUAL(hash_func(ue1), hash_func(ue1_));
+	CPPUNIT_ASSERT_EQUAL(hash_func(ue2), hash_func(ue2_));
+	CPPUNIT_ASSERT_EQUAL(hash_func(ue3), hash_func(ue3_));
 }
 
diff --git a/alib2data/test-src/graph/GraphElementsTest.h b/alib2data/test-src/graph/GraphElementsTest.h
index 279c89c2b8..31c4d1311d 100644
--- a/alib2data/test-src/graph/GraphElementsTest.h
+++ b/alib2data/test-src/graph/GraphElementsTest.h
@@ -11,11 +11,15 @@ class GraphElementsTest : public CppUnit::TestFixture
 	CPPUNIT_TEST_SUITE(GraphElementsTest);
 	CPPUNIT_TEST(testCopyConstruct);
 	CPPUNIT_TEST(testEqual);
+	CPPUNIT_TEST(testLessThan);
+	CPPUNIT_TEST(testHash);
 	CPPUNIT_TEST_SUITE_END();
 
 public:
 	void testCopyConstruct();
 	void testEqual();
+	void testLessThan();
+	void testHash();
 };
 
 #endif	// GRAPH_ELEMENTS_TEST_H_
diff --git a/alib2data/test-src/graph/GraphTest.cpp b/alib2data/test-src/graph/GraphTest.cpp
index 52132ffe5d..5352db77fd 100644
--- a/alib2data/test-src/graph/GraphTest.cpp
+++ b/alib2data/test-src/graph/GraphTest.cpp
@@ -804,6 +804,12 @@ void GraphTest::testHasNodeAndEdge_impl(graph::REPRESENTATION representation)
 	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(ue4));
 	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(ue5));
 	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(ue6));
+	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(graph::UndirectedEdge(n2, n1)));
+	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(graph::UndirectedEdge(n3, n1)));
+	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(graph::UndirectedEdge(n4, n1)));
+	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(graph::UndirectedEdge(n5, n1)));
+	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(graph::UndirectedEdge(n5, n2)));
+	CPPUNIT_ASSERT_EQUAL(true, ug.hasEdge(graph::UndirectedEdge(n6, n5)));
 	CPPUNIT_ASSERT_EQUAL(false, ug.hasEdge(graph::UndirectedEdge(n1, n6)));
 	CPPUNIT_ASSERT_EQUAL(false, ug.hasEdge(graph::UndirectedEdge(n2, n6)));
 	CPPUNIT_ASSERT_EQUAL(false, ug.hasEdge(graph::UndirectedEdge(n3, n6)));
-- 
GitLab