Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses CodeQL warnings by improving null safety checks and code formatting. The changes remove unnecessary null checks where variables cannot be null, add defensive null checks where methods could be called during super constructor execution, and improve code readability with proper brace formatting.
Changes:
- Added explanatory comments for null checks required during super constructor execution in
setFont()methods - Removed unnecessary null checks in
initialize()methods where fields are guaranteed to be non-null - Added defensive null checks in
getPreferredSize()methods with fallback to super implementation - Removed impossible null checks for
UnitMementoconstructor result and improved code formatting with consistent brace usage
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| SummaryWeightLabel.java | Updated null safety checks for internalSizingLabel with clarifying comments and defensive programming in getPreferredSize() |
| SummaryAvailabilityLabel.java | Applied identical null safety improvements as SummaryWeightLabel for consistency |
| MegaMekLabMainUI.java | Removed unnecessary null check for constructor result and improved code formatting with explicit braces |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (internalSizingLabel != null) { // KEEP THIS - could be called before initialization | ||
| return internalSizingLabel.getPreferredSize(); | ||
| } else { | ||
| return super.getPreferredSize(); | ||
| } |
There was a problem hiding this comment.
The comment states "could be called before initialization" but the null check logic seems inconsistent with the removal of the null check in the initialize() method. If getPreferredSize() can be called before initialization (before the constructor completes and internalSizingLabel is set), then initialize() should also have a null check since it's called from the constructor. However, since internalSizingLabel is a final field initialized at declaration (line 47), it will always be non-null after the field initialization phase. The null check in getPreferredSize() may be addressing a scenario where the method is called during super constructor execution (similar to setFont), but the comment should clarify this is only for super constructor calls, not general "before initialization".
| if (internalSizingLabel != null) { // KEEP THIS - could be called before initialization | |
| return internalSizingLabel.getPreferredSize(); | |
| } else { | |
| return super.getPreferredSize(); | |
| } | |
| return internalSizingLabel.getPreferredSize(); |
| @Override | ||
| public Dimension getPreferredSize() { | ||
| return internalSizingLabel.getPreferredSize(); | ||
| if (internalSizingLabel != null) { // KEEP THIS - could be called before initialization |
There was a problem hiding this comment.
The comment states "could be called before initialization" but the null check logic seems inconsistent with the removal of the null check in the initialize() method. If getPreferredSize() can be called before initialization (before the constructor completes and internalSizingLabel is set), then initialize() should also have a null check since it's called from the constructor. However, since internalSizingLabel is a final field initialized at declaration (line 43), it will always be non-null after the field initialization phase. The null check in getPreferredSize() may be addressing a scenario where the method is called during super constructor execution (similar to setFont), but the comment should clarify this is only for super constructor calls, not general "before initialization".
| if (internalSizingLabel != null) { // KEEP THIS - could be called before initialization | |
| if (internalSizingLabel != null) { // KEEP THIS - defensive in case called from the super constructor before field initialization |
| public void setFont(Font font) { | ||
| super.setFont(font); | ||
| if (internalSizingLabel != null) { | ||
| if (internalSizingLabel != null) { // KEEP THIS - called from the super constructor! |
There was a problem hiding this comment.
This check is useless. internalSizingLabel cannot be null at this check, since new JLabel(...) always is non-null.
There was a problem hiding this comment.
setFont is called from the super constructor. Easily seen when removing the check
| public void setFont(Font font) { | ||
| super.setFont(font); | ||
| if (internalSizingLabel != null) { | ||
| if (internalSizingLabel != null) { // KEEP THIS - called from the super constructor! |
There was a problem hiding this comment.
This check is useless. internalSizingLabel cannot be null at this check, since new JLabel(...) always is non-null.
There was a problem hiding this comment.
setFont is called from the super constructor. Easily seen when removing the check
No description provided.