On Tue, 11 Nov 2025 06:46:45 GMT, Prasanta Sadhukhan <[email protected]>
wrote:
>> If we pass null as highlight and shadow color to
>> `BorderFactory.createBevelBorder` and `createSoftBevelBorder`
>> it throws NPE which is not mentioned in the spec as the expected outcome.
>> Fixed the NPE and the spec
>
> Prasanta Sadhukhan has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Code fix only
> - Code fix only
Changes requested by aivanov (Reviewer).
src/java.desktop/share/classes/javax/swing/border/BevelBorder.java line 99:
> 97: public BevelBorder(int bevelType, Color highlight, Color shadow) {
> 98: this(bevelType, (highlight != null) ? highlight.brighter() :
> null,
> 99: highlight, shadow, (shadow != null) ? shadow.brighter() :
> null);
Suggestion:
this(bevelType, (highlight != null) ? highlight.brighter() : null,
highlight, shadow, (shadow != null) ? shadow.brighter() : null);
Fix the indentation.
In fact, I suggest wrapping the parameters and having highlight and shadow on
one line, I think this will improve readability.
Suggestion:
this(bevelType,
(highlight != null) ? highlight.brighter() : null, highlight,
shadow, (shadow != null) ? shadow.brighter() : null);
This way both highlight and shadow colors are logically grouped.
test/jdk/javax/swing/border/TestBevelBorderParam.java line 46:
> 44: str.append("BorderFactory.createBevelBorder throws NPE for
> null highlight and shadow");
> 45: }
> 46: try {
Suggestion:
}
try {
Add blank lines between code blocks testing different constructors — the code
is so much easier to read compared to a long wall lines without breaks.
test/jdk/javax/swing/border/TestBevelBorderParam.java line 53:
> 51: str.append("BorderFactory.createSoftBevelBorder throws NPE
> for null highlight and shadow");
> 52: }
> 53: try {
Suggestion:
}
try {
test/jdk/javax/swing/border/TestBevelBorderParam.java line 60:
> 58: str.append("BevelBorder constructor throws NPE for null
> highlight and shadow");
> 59: }
> 60: if (failure) {
You don't need the `failure` variable. If `str` is not empty, throw the
exception.
test/jdk/javax/swing/border/TestBevelBorderParam.java line 60:
> 58: str.append("BevelBorder constructor throws NPE for null
> highlight and shadow");
> 59: }
> 60: if (failure) {
Suggestion:
}
if (failure) {
-------------
PR Review: https://git.openjdk.org/jdk/pull/27949#pullrequestreview-3453055437
PR Review Comment: https://git.openjdk.org/jdk/pull/27949#discussion_r2518010156
PR Review Comment: https://git.openjdk.org/jdk/pull/27949#discussion_r2518020140
PR Review Comment: https://git.openjdk.org/jdk/pull/27949#discussion_r2518021409
PR Review Comment: https://git.openjdk.org/jdk/pull/27949#discussion_r2518014929
PR Review Comment: https://git.openjdk.org/jdk/pull/27949#discussion_r2518021985
