From 43583530fd35284b6b72ae597a53e9dbdd410800 Mon Sep 17 00:00:00 2001 From: Kay Stenschke Date: Mon, 27 Mar 2017 23:41:26 +0200 Subject: [PATCH 1/3] Update NaturalOrderComparator.java Minor modernization of code style --- NaturalOrderComparator.java | 98 +++++++++++++------------------------ 1 file changed, 34 insertions(+), 64 deletions(-) diff --git a/NaturalOrderComparator.java b/NaturalOrderComparator.java index b3040db..4e82e48 100644 --- a/NaturalOrderComparator.java +++ b/NaturalOrderComparator.java @@ -28,9 +28,7 @@ public class NaturalOrderComparator implements Comparator { int compareRight(String a, String b) { - int bias = 0; - int ia = 0; - int ib = 0; + int bias = 0, ia = 0, ib = 0; // The longest run of digits wins. That aside, the greatest // value wins, but we can't know that it will until we've scanned @@ -41,33 +39,27 @@ int compareRight(String a, String b) char ca = charAt(a, ia); char cb = charAt(b, ib); - if (!Character.isDigit(ca) && !Character.isDigit(cb)) - { + if (!Character.isDigit(ca) && !Character.isDigit(cb)) { return bias; } - if (!Character.isDigit(ca)) - { + if (!Character.isDigit(ca)) { return -1; } - if (!Character.isDigit(cb)) - { + if (!Character.isDigit(cb)) { return +1; } - if (ca < cb) - { - if (bias == 0) - { + if (ca == 0 && cb == 0) { + return bias; + } + + if (ca < cb) { + if (bias == 0) { bias = -1; } - } - else if (ca > cb) - { - if (bias == 0) + } else if (ca > cb) { + if (bias == 0) { bias = +1; - } - else if (ca == 0 && cb == 0) - { - return bias; + } } } } @@ -82,67 +74,51 @@ public int compare(Object o1, Object o2) char ca, cb; int result; - while (true) - { - // only count the number of zeroes leading the last number compared + while (true) { + // Only count the number of zeroes leading the last number compared nza = nzb = 0; ca = charAt(a, ia); cb = charAt(b, ib); // skip over leading spaces or zeros - while (Character.isSpaceChar(ca) || ca == '0') - { - if (ca == '0') - { + while (Character.isSpaceChar(ca) || ca == '0') { + if (ca == '0') { nza++; - } - else - { - // only count consecutive zeroes + } else { + // Only count consecutive zeroes nza = 0; } ca = charAt(a, ++ia); } - while (Character.isSpaceChar(cb) || cb == '0') - { - if (cb == '0') - { + while (Character.isSpaceChar(cb) || cb == '0') { + if (cb == '0') { nzb++; - } - else - { - // only count consecutive zeroes + } else { + // Only count consecutive zeroes nzb = 0; } cb = charAt(b, ++ib); } - // process run of digits - if (Character.isDigit(ca) && Character.isDigit(cb)) - { - if ((result = compareRight(a.substring(ia), b.substring(ib))) != 0) - { - return result; - } + // Process run of digits + if (Character.isDigit(ca) && Character.isDigit(cb) && ((result = compareRight(a.substring(ia), b.substring(ib))) != 0)) { + return result; } - if (ca == 0 && cb == 0) - { + if (ca == 0 && cb == 0) { // The strings compare the same. Perhaps the caller // will want to call strcmp to break the tie. return nza - nzb; } - if (ca < cb) - { + if (ca < cb) { return -1; } - if (ca > cb) - { + if (ca > cb) { return +1; } @@ -151,22 +127,16 @@ public int compare(Object o1, Object o2) } } - static char charAt(String s, int i) - { - if (i >= s.length()) - { - return 0; - } - - return s.charAt(i); + static char charAt(String s, int i) { + return i >= s.length() ? 0 : s.charAt(i); } public static void main(String[] args) { String[] strings = new String[] { "1-2", "1-02", "1-20", "10-20", "fred", "jane", "pic01", - "pic2", "pic02", "pic02a", "pic3", "pic4", "pic 4 else", "pic 5", "pic05", "pic 5", - "pic 5 something", "pic 6", "pic 7", "pic100", "pic100a", "pic120", "pic121", - "pic02000", "tom", "x2-g8", "x2-y7", "x2-y08", "x8-y8" }; + "pic2", "pic02", "pic02a", "pic3", "pic4", "pic 4 else", "pic 5", "pic05", "pic 5", + "pic 5 something", "pic 6", "pic 7", "pic100", "pic100a", "pic120", "pic121", + "pic02000", "tom", "x2-g8", "x2-y7", "x2-y08", "x8-y8" }; List orig = Arrays.asList(strings); From 000212c129f36c3be6498b0d3830f0141ea74c48 Mon Sep 17 00:00:00 2001 From: Kay Stenschke Date: Tue, 28 Mar 2017 09:42:51 +0200 Subject: [PATCH 2/3] merged formerly overlooked nested positive if, clarified potentially confusing declaration that was inlined in condition by separating it from the condition and using a "speaking" variable name that continous the afore used wording --- NaturalOrderComparator.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/NaturalOrderComparator.java b/NaturalOrderComparator.java index 4e82e48..f9a7cef 100644 --- a/NaturalOrderComparator.java +++ b/NaturalOrderComparator.java @@ -56,10 +56,8 @@ int compareRight(String a, String b) if (bias == 0) { bias = -1; } - } else if (ca > cb) { - if (bias == 0) { - bias = +1; - } + } else if (ca > cb && bias == 0) { + bias = +1; } } } @@ -72,7 +70,6 @@ public int compare(Object o1, Object o2) int ia = 0, ib = 0; int nza = 0, nzb = 0; char ca, cb; - int result; while (true) { // Only count the number of zeroes leading the last number compared @@ -105,8 +102,11 @@ public int compare(Object o1, Object o2) } // Process run of digits - if (Character.isDigit(ca) && Character.isDigit(cb) && ((result = compareRight(a.substring(ia), b.substring(ib))) != 0)) { - return result; + if (Character.isDigit(ca) && Character.isDigit(cb)) { + int bias = compareRight(a.substring(ia), b.substring(ib)); + if (bias != 0) { + return bias; + } } if (ca == 0 && cb == 0) { @@ -114,7 +114,6 @@ public int compare(Object o1, Object o2) // will want to call strcmp to break the tie. return nza - nzb; } - if (ca < cb) { return -1; } From 65bd9f428149deedf6457fabcd0ec6acfcbf1557 Mon Sep 17 00:00:00 2001 From: Kay Stenschke Date: Tue, 28 Mar 2017 10:35:56 +0200 Subject: [PATCH 3/3] Singled out duplicated condition --- NaturalOrderComparator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/NaturalOrderComparator.java b/NaturalOrderComparator.java index f9a7cef..3a6064c 100644 --- a/NaturalOrderComparator.java +++ b/NaturalOrderComparator.java @@ -52,12 +52,12 @@ int compareRight(String a, String b) return bias; } - if (ca < cb) { - if (bias == 0) { + if (bias == 0) { + if (ca < cb) { bias = -1; + } else if (ca > cb) { + bias = +1; } - } else if (ca > cb && bias == 0) { - bias = +1; } } }