diff --git a/src/App/MappedElement.cpp b/src/App/MappedElement.cpp index 14eb6f99ba..25e7abf7cc 100644 --- a/src/App/MappedElement.cpp +++ b/src/App/MappedElement.cpp @@ -22,3 +22,136 @@ #include "PreCompiled.h" #include "MappedElement.h" + +using namespace Data; + +bool ElementNameComparator::operator()(const MappedName& leftName, + const MappedName& rightName) const +{ + int size = static_cast(std::min(leftName.size(), rightName.size())); + if (size == 0U) { + return leftName.size() < rightName.size(); + } + int currentIndex = 0; + if (rightName[0] == '#') { + if (leftName[0] != '#') { + return true; + } + // If both string starts with '#', compare the following hex digits by + // its integer value. + int res = 0; + for (currentIndex = 1; currentIndex < size; ++currentIndex) { + auto ac = (unsigned char)leftName[currentIndex]; + auto bc = (unsigned char)rightName[currentIndex]; + if (std::isxdigit(bc) != 0) { + if (std::isxdigit(ac) == 0) { + return true; + } + if (res == 0) { + if (ac < bc) { + res = -1; + } + else if (ac > bc) { + res = 1; + } + } + } + else if (std::isxdigit(ac) != 0) { + res = 1; + } + else { + break; + } + } + if (res < 0) { + return true; + } + if (res > 0) { + return false; + } + + for (; currentIndex < size; ++currentIndex) { + char ac = leftName[currentIndex]; + char bc = rightName[currentIndex]; + if (ac < bc) { + return true; + } + if (ac > bc) { + return false; + } + } + return leftName.size() < rightName.size(); + } + if (leftName[0] == '#') { + return false; + } + + // If the string does not start with '#', compare the non-digits prefix + // using lexical order. + for (currentIndex = 0; currentIndex < size; ++currentIndex) { + auto ac = (unsigned char)leftName[currentIndex]; + auto bc = (unsigned char)rightName[currentIndex]; + if (std::isdigit(bc) == 0) { + if (std::isdigit(ac) != 0) { + return true; + } + if (ac < bc) { + return true; + } + if (ac > bc) { + return false; + } + } + else if (std::isdigit(ac) == 0) { + return false; + } + else { + break; + } + } + + // Then compare the following digits part by integer value + int res = 0; + for (; currentIndex < size; ++currentIndex) { + auto ac = (unsigned char)leftName[currentIndex]; + auto bc = (unsigned char)rightName[currentIndex]; + if (std::isdigit(bc) != 0) { + if (std::isdigit(ac) == 0) { + return true; + } + if (res == 0) { + if (ac < bc) { + res = -1; + } + else if (ac > bc) { + res = 1; + } + } + } + else if (std::isdigit(ac) != 0) { + return false; + } + else { + break; + } + } + if (res < 0) { + return true; + } + if (res > 0) { + return false; + } + + // Finally, compare the remaining tail using lexical order + for (; currentIndex < size; ++currentIndex) { + char ac = leftName[currentIndex]; + char bc = rightName[currentIndex]; + if (ac < bc) { + return true; + } + if (ac > bc) { + return false; + } + } + return leftName.size() < rightName.size(); +} \ No newline at end of file diff --git a/src/App/MappedElement.h b/src/App/MappedElement.h index 7f6ee68f01..798092176d 100644 --- a/src/App/MappedElement.h +++ b/src/App/MappedElement.h @@ -98,6 +98,22 @@ struct AppExport MappedElement } }; +struct AppExport ElementNameComparator { + /** Comparison function to make topo name more stable + * + * The sorting decomposes the name into either of the following two forms + * '#' + hex_digits + tail + * non_digits + digits + tail + * + * The non-digits part is compared lexically, while the digits part is + * compared by its integer value. + * + * The reason for this is to prevent names with bigger digits (which usually means + * they come later in history) from coming earlier when sorting. + */ + bool operator()(const MappedName & leftName, const MappedName & rightName) const; +}; + }// namespace Data diff --git a/tests/src/App/MappedElement.cpp b/tests/src/App/MappedElement.cpp index 0ea03de6af..36588a8d92 100644 --- a/tests/src/App/MappedElement.cpp +++ b/tests/src/App/MappedElement.cpp @@ -127,3 +127,91 @@ TEST_F(MappedElementTest, lessThanOperator) EXPECT_FALSE(mappedElement2A < mappedElement1B); EXPECT_FALSE(mappedElement2B < mappedElement2BDuplicate); } + +TEST_F(MappedElementTest, comparatorBothAreZeroSize) +{ + // Arrange + Data::MappedName mappedName1 {""}; + Data::MappedName mappedName2 {""}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_FALSE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, comparatorOneIsZeroSize) +{ + // Arrange + Data::MappedName mappedName1 {""}; + Data::MappedName mappedName2 {"#12345"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_TRUE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, comparatorBothStartWithHexDigitsThatDiffer) +{ + // Arrange + Data::MappedName mappedName1 {"#fed;B"}; + Data::MappedName mappedName2 {"#abcdef;A"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_TRUE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, comparatorBothStartWithTheSameHexDigits) +{ + // Arrange + Data::MappedName mappedName1 {"#12345;B"}; + Data::MappedName mappedName2 {"#12345;A"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_FALSE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, DISABLED_comparatorHexWithoutTerminatorIsBroken) +{ + // Arrange + Data::MappedName mappedName1 {"#fed"}; + Data::MappedName mappedName2 {"#abcdef"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_FALSE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, comparatorNoHexDigitsLexicalCompare) +{ + // Arrange + Data::MappedName mappedName1 {"A"}; + Data::MappedName mappedName2 {"B"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_TRUE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, comparatorNoHexDigitsSameStringNumericCompare) +{ + // Arrange + Data::MappedName mappedName1 {"Edge123456;"}; + Data::MappedName mappedName2 {"Edge321;"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_FALSE(comp(mappedName1, mappedName2)); +} + +TEST_F(MappedElementTest, DISABLED_comparatorIntegerWithoutTerminatorIsBroken) +{ + // Arrange + Data::MappedName mappedName1 {"Edge123456"}; + Data::MappedName mappedName2 {"Edge321"}; + auto comp = Data::ElementNameComparator(); + + // Act & Assert + EXPECT_FALSE(comp(mappedName1, mappedName2)); +}