Mike Kolesnik has posted comments on this change.
Change subject: core: Added LexoNumericComparator utility
......................................................................
Patch Set 1: (4 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/LexoNumericComparator.java
Line 14: * <br>
Line 15: * It is assumed that a string always begins with a nondigit sequence;
if it actually begins with a digit sequence, the
Line 16: * behaviour is as if it started with an empty nondigit sequence.
Line 17: */
Line 18: public class LexoNumericComparator implements Comparator<String> {
It is not out convention to write comments in line (across this class)
Line 19:
Line 20: @Override
Line 21: public int compare(String str1, String str2) {
Line 22: return comp(str1, str2);
Line 28: } else if (str2 == null) {
Line 29: return 1;
Line 30: }
Line 31:
Line 32: boolean isDigitTurn = false;
It is not our convention to name boolean variables with "is" prefix.
Line 33: int begSeq1 = 0, begSeq2 = 0, endSeq1, endSeq2;
Line 34:
Line 35: while ((endSeq1 = findEndOfSequence(str1, begSeq1,
isDigitTurn)) != begSeq1) {
Line 36: if ((endSeq2 = findEndOfSequence(str2, begSeq2,
isDigitTurn)) == begSeq2) {
Line 29: return 1;
Line 30: }
Line 31:
Line 32: boolean isDigitTurn = false;
Line 33: int begSeq1 = 0, begSeq2 = 0, endSeq1, endSeq2;
Why initialize primitives to their defauls values?
Also it's not out convention to declare multiple variables in a single
statement.
Line 34:
Line 35: while ((endSeq1 = findEndOfSequence(str1, begSeq1,
isDigitTurn)) != begSeq1) {
Line 36: if ((endSeq2 = findEndOfSequence(str2, begSeq2,
isDigitTurn)) == begSeq2) {
Line 37: return 1; // str1 and str2 have the same prefix but
str1 has an extra sequence => str1 > str2
Line 61: }
Line 62:
Line 63: private static int compDigitSequence(String seq1, String seq2) {
Line 64: int compRes = ((Integer)
Integer.parseInt(seq1)).compareTo(Integer.parseInt(seq2));
Line 65: return compRes != 0 ? compRes : compNonDigitSequence(seq1,
seq2);
Perhaps it would be easier to read a positive check than a negative one?
Line 66: }
Line 67:
Line 68: private static int compNonDigitSequence(String seq1, String seq2) {
Line 69: return seq1.compareTo(seq2);
--
To view, visit http://gerrit.ovirt.org/11996
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c68446ea6f1865a51455a0a359df339bfb15bba
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
