-
-
Notifications
You must be signed in to change notification settings - Fork 27.3k
#587 sonarqube bugs #599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#587 sonarqube bugs #599
Conversation
* | ||
*/ | ||
public class Client extends JFrame { | ||
public class Client extends JFrame { // NOSONAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is // NOSONAR
needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sonarqube reported "This class has 6 parents which is greater than 5 authorized"
Here is the inheritance hierarchy.
FileSelectorJFrame -> JFrame -> Frame -> Window -> Container -> Component implements ImageObserver, MenuContainer, Serializable
I believe, it is most common to subclass JFrame for creating window.
Alternate design by following "Use composition over inheritance". I thought it would make more changes and may spoil the brevity of explaining the design pattern.
Please let me know if you want me to think about the alternate design
private static final long serialVersionUID = 1L; | ||
|
||
private FilterManager filterManager; | ||
private transient FilterManager filterManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making it transient doesn't introduce any problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It did not introduce any problem.
Alternate solution would be making FilterManager and its parents as Serializable. In this example, we are not transferring Client instance or planning to serialize, So I decided to make it transient to keep it simple.
Let me know if you prefer marking all classes as Serializable as long as it does not have any non-serialized attributes.
* | ||
*/ | ||
public class Target extends JFrame { | ||
public class Target extends JFrame { //NOSONAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is // NOSONAR
needed here?
@mookkiah thanks for the explanations. It looks good to be merged. Well done 👍 |
No description provided.