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

api: abstract classes that extending concrete class #4716

Closed
romani opened this Issue Jul 13, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@romani
Member

romani commented Jul 13, 2017

from Intellij Idea inspection - AbstractClassExtendsConcreteClass

Inheritance issues group 4 errors
Abstract class extends concrete class inspection 3 errors
class AbstractLoader 1 error
ERROR Class 'AbstractLoader' is declared 'abstract', and extends a concrete class
class AbstractViolationReporter 1 error
ERROR Class 'AbstractViolationReporter' is declared 'abstract', and extends a concrete class

We need to review such cases and make a decision how to refactor this bad design.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jul 14, 2017

Member

All the classes mentioned extend AutomaticBean which is also trying to be refactored at #3542 .

Member

rnveach commented Jul 14, 2017

All the classes mentioned extend AutomaticBean which is also trying to be refactored at #3542 .

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Aug 27, 2017

Member

@romani After looking at the class from working on the other issue, I think we should just make it abstract with no abstract methods.
Most of our other abstract classes have empty method definitions that can be overridden by demand or left as an empty implementation. I don't really see any method that should be made abstract.

Member

rnveach commented Aug 27, 2017

@romani After looking at the class from working on the other issue, I think we should just make it abstract with no abstract methods.
Most of our other abstract classes have empty method definitions that can be overridden by demand or left as an empty implementation. I don't really see any method that should be made abstract.

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 5, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 9, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 10, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 10, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 10, 2017

romani added a commit that referenced this issue Dec 13, 2017

@romani romani added this to the 8.6 milestone Dec 13, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 13, 2017

Member

suppressions are merged, actual fixes in classes are postponed to other issues:
Rename AutomaticBean #5337
Move, Rename, and remove abstract from AbstractLoader #5338

Member

romani commented Dec 13, 2017

suppressions are merged, actual fixes in classes are postponed to other issues:
Rename AutomaticBean #5337
Move, Rename, and remove abstract from AbstractLoader #5338

@romani romani closed this Dec 13, 2017

timurt added a commit to timurt/checkstyle that referenced this issue Dec 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment