vesterstroem commented on a change in pull request #174:
URL: https://github.com/apache/commons-text/pull/174#discussion_r499133321
##########
File path:
src/main/java/org/apache/commons/text/similarity/LevenshteinDistance.java
##########
@@ -285,6 +286,11 @@ private static int limitedCompare(CharSequence left,
CharSequence right, final i
// left and up
d[i] = 1 + Math.min(Math.min(d[i - 1], p[i]), p[i - 1]);
}
+ lowerBound = Math.min(lowerBound, d[i]);
+ }
+ // if the lower bound is greater than the threshold, then exit
early
+ if (lowerBound > threshold) {
+ return -1;
Review comment:
The improvement introduces the need for an extra test, which I forgot to
add.
The assertion:
assertThat(new LevenshteinDistance(1).apply("abc",
"acb")).isEqualTo(-1);
should be inserted into e.g. testGetLevenshteinDistance_StringStringInt of
LevenshteinDistanceTest.
That will give coverage to the uncovered line - and it will fail, if we just
always return p[n].
Should I make a new pull request or update the existing one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
