[TEXT-190] Document negative limit for WordUtils abbreviate method #181
Conversation
lower < 0 has no defined behavior in the document
| @@ -859,7 +859,7 @@ public static boolean isDelimiter(final int codePoint, final char[] delimiters) | |||
| public static String abbreviate(final String str, int lower, int upper, final String appendToEnd) { | |||
| Validate.isTrue(upper >= -1, "upper value cannot be less than -1"); | |||
| Validate.isTrue(upper >= lower || upper == -1, "upper value is less than lower value"); | |||
|
|
|||
| Validate.isTrue(lower >= 0, "lower value cannot be lower than 0"); | |||
There was a problem hiding this comment.
@bjmbing thanks for the pull request. Initially this change made sense to me. Then looking at the abbreviate code, looks like the lower limit is used for StringUtils.indexOf(str, " ", /* startPos */ lower).
StringUtils.indexOf javadocs say: "* @param startPos the start position, negative treated as zero".
Currently, if you pass -1 or any other negative value, it is treated as zero, in accord with the StringUtils.indexOf javadocs.
I think a better solution would be to document that negative values are treated as zero, essentially replicating the StringUtils docs. This way, existing code using negative values won't get a runtime exception (which Validate.isTrue would do). What do you think?
There was a problem hiding this comment.
Yeah I think the solution you provided is good.
There was a problem hiding this comment.
Thanks for the prompt reply @bjmbing! If you update this PR with something similar to the javadocs from StringUtils for the lower limit, we can then create a JIRA issue, and merge this PR :-)
There was a problem hiding this comment.
Hi @kinow . I have updated the javadoc for lower in WordUtils.abbreviate.
| @@ -828,7 +828,7 @@ public static boolean isDelimiter(final int codePoint, final char[] delimiters) | |||
| * | |||
| * @param str the string to be abbreviated. If null is passed, null is returned. | |||
| * If the empty String is passed, the empty string is returned. | |||
| * @param lower the lower limit. | |||
| * @param lower the lower limit; < -1 value is treated as 0. | |||
There was a problem hiding this comment.
Might be better ", negative treated as zero", as any negative value, not only -1 is treated as 0
kinow
changed the title
|
I've created https://issues.apache.org/jira/browse/TEXT-190 for this issue, updated the title. Once the PR passes review, we should be ready to merge and include in the changelog 👍 Thanks @bjmbing ! |
|
I changed the doc as suggested. @kinow . Thank you for your help |
lower < 0 has no defined behavior in the document. Although it works similar to lower = 0. There is no related document for it.