-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Make class abstract driver #16347
Make class abstract driver #16347
Conversation
What is the logic of this refactoring? I mean I do not really understand what is an abstract class in pharo and when i will need it. |
… #isAbstract. Migrate RBMakeClassAbstractTransformation. Add data for testing: #RBClassWithoutSelfClassReferenceTest and #RBWithSelfClassReferenceTest Update RBMakeClassAbstractParametrizedTest Add #RBMakeClassAbstractDriver with tests Update #SycCMakeAbstractCommand
I migrated because it was there and some people may be using it. However, I never bought the arguments for "improving code clarity" or "preventing improper instantiation" (maybe they're good for educational purposes), so I'll provide an example where I used it some time ago as a dirty experiment: To migrate a legacy system, using automated scripts, some classes couldn't be migrated to the new system. To get a quick diff of which ones weren't migrated, I annotated the already migrated abstract classes with the "isAbstract" method, so I could later see which hierarchies have fewer subclasses than expected. If there are other use cases I would have to think about them because as you said, there doesn't seem to be too many. |
I use it a lot, it is useful for the Browser and the Rules to know that a class is abstract. Imagine having a subclassResponsibility method. The browser nags you to implement it, but the subclass is Abtract, too. So we could add the method again, then it complaints that the method is the same as in the superclass. => add #isAbstract, and the Browser/Rules know that you do not have to implement the #subclassResponsability here and just ask you to implement it where you want it |
I also like to know easily when a class is abstract when I'm exploring a project I'm not familiar with. Like that I'm able to understand the structure of the project more easily. And I know that I should not instantiate those classes but just look at the subclasses. |
src/Refactoring-Transformations-Tests/RBAbstractRefactoringTest.class.st
Outdated
Show resolved
Hide resolved
src/Refactoring-UI-Tests/ReMakeClassAbstractDriverTest.class.st
Outdated
Show resolved
Hide resolved
Co-authored-by: balsa-sarenac <34557616+balsa-sarenac@users.noreply.github.com>
Nice one! Co-authored-by: balsa-sarenac <34557616+balsa-sarenac@users.noreply.github.com>
CI got a random failure. I'm restarting it |
Some tests are refactoring related so it's possible it comes from here no? |
Ok, I fixed the failing tests. Now it seems good to go |
This PR implements the "Make Abstract" class refactoring using the new Driver interface.
Make RBAbstractClass match the MOP answering both false by default in #isAbstract.
Migrate RBMakeClassAbstractTransformation.
Add data for testing: #RBClassWithoutSelfClassReferenceTest and #RBWithSelfClassReferenceTest
Update RBMakeClassAbstractParametrizedTest
Add #RBMakeClassAbstractDriver with tests
Update #SycCMakeAbstractCommand