-
Notifications
You must be signed in to change notification settings - Fork 737
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
Replace public constructor on an abstract class with protected #776
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
698e400
to
d9f25a0
Compare
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.
Makes sense :)
@BugPattern( | ||
name = "PublicConstructorForAbstractClass", | ||
summary = | ||
"Constructor on an abstract class can be declared abstract as there is never a need it to be public", |
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.
s/declared abstract/declared protected/
s/need it/need for it/
testHelper.addInputLines( | ||
"in/Test.java", | ||
"public abstract class Test {", | ||
" public Test() { }", |
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.
Maybe use two-space indentation in the test code?
d9f25a0
to
7eb9fd8
Compare
Ping for review. Already addressed comments by Stephen |
protected Fixes #776 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=312188474
protected Fixes #776 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=312188474
Fixes - #134. Added a suggestion to modify the modifier of constructor in an abstract class to protected. The constructor in abstract class doesn't need to be public since it can't be instantiated and it leads to better code semantics.