LANG-1720: Fix Javadoc description of exceptions thrown for existing classes #1160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elharo
suggested changes
Jan 17, 2024
| @@ -280,6 +280,9 @@ public static char binaryToHexDigitMsb0_4bits(final boolean[] src, final int src | |||
| if (src.length - srcPos < 4) { | |||
| throw new IllegalArgumentException("src.length-srcPos<4: src.length=" + src.length + ", srcPos=" + srcPos); | |||
| } | |||
| if (srcPos >= 0) { | |||
| throw new IllegalArgumentException("srcPos>=0: srcPos=" + srcPos); | |||
There was a problem hiding this comment.
This is not api doc. It's a new exception, and I'm not sure it's correct.
| @@ -538,6 +538,7 @@ public String replace(final char[] source) { | |||
| * @param offset the start offset within the array, must be valid | |||
| * @param length the length within the array to be processed, must be valid | |||
| * @return the result of the replace operation | |||
| * @throws StringIndexOutOfBoundsException if {@code offset < 0 || offset + length >= source.length} | |||
There was a problem hiding this comment.
Why String instead of array here? If this is the behavior it might be a bug. Change the doc to just IndexOutOfBoundsException for now in case we need to fix this.
| @@ -577,6 +578,7 @@ public String replace(final CharSequence source) { | |||
| * @param offset the start offset within the array, must be valid | |||
| * @param length the length within the array to be processed, must be valid | |||
| * @return the result of the replace operation | |||
| * @throws StringIndexOutOfBoundsException if {@code offset < 0 || offset + length >= source.length} | |||
There was a problem hiding this comment.
This should maybe be IndexOutOfBoundsException
| @@ -753,6 +757,7 @@ public boolean replaceIn(final StrBuilder source) { | |||
| * @param offset the start offset within the array, must be valid | |||
| * @param length the length within the builder to be processed, must be valid | |||
| * @return true if altered | |||
| * @throws StringIndexOutOfBoundsException if {@code offset < 0 || offset + length >= source.length} | |||
There was a problem hiding this comment.
It does strike me as possible that some of these exceptions shouldn't be thrown at all. If offset or length are out of bounds, maybe nothing is replaced.
|
Hm, where are we on this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During my usage of commons-lang, I have noticed some exceptions (in
StringUtils,StrSubstitutor,CharSequenceTranslatorandConversion) that are not documented in the Javadoc, which I consider to be unhandled exceptions.Before creating my this PR, I noticed #1139, which addresses similar issues. In order to avoid conflicts and show my respect, I have removed some of the Javadoc comments I added in
Conversion.java, aligning with PR #1139.