Skip to content
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

Remove explicitRequirement #80

Open
JanBliznicenko opened this issue Nov 9, 2024 · 3 comments
Open

Remove explicitRequirement #80

JanBliznicenko opened this issue Nov 9, 2024 · 3 comments

Comments

@JanBliznicenko
Copy link
Contributor

explicitRequirement is deprecated in Pharo 13 because of being cause of slowdowns and bugs due to its complexity:
pharo-project/pharo#17276

In Roassal, it is used in multiple places:
https://github.com/search?q=repo%3Apharo-graphics%2FRoassal+%22self+explicitRequirement%22&type=code

It should be replaced, possibly by a super send?

@tinchodias
Copy link
Contributor

May self subclassResponsability be a replacement? or leave the method empty. I suspect they define API that all users define.

Didn't check all. But for example, in a fresh Pharo 13 (so old version of Roassal), but I browsed RSTContainer>>#fixedShapes and see that both users override.

IIRC Milton one year ago replaced relevant uses of explicitRequirement in Roassal. I mean, the ones that were actually executed and then were a real slowdown.

@JanBliznicenko
Copy link
Contributor Author

JanBliznicenko commented Nov 12, 2024

Well, the original explicitRequirement is more like a super send, as it is designed to work even if the method is actually implemented in a superclass, instead of the traited class or a subclass. RSLinePlot uses the behavior. Its method

shape
    ^ self explicitRequirement

actually calls method shape from its superclass. It would not be possible with subclassResponsibility. If I just replace the explicitRequirement by subclassResponsibility in RSTLine, RSLinePlot breaks (along with all its tests).

However implementing the method to just call super feels wrong. The closes to the explicitRequirement is probably shouldBeImplemented (closest semantically) and in case of RSLinePlot, it would need to be overridden to call super or excluded from the trait usage.

@tinchodias
Copy link
Contributor

Interesting points. What about removing shape? in one hand, we know that trait users already define shape and in the other, (at least in my Pharo 12) calypso doesn't highlight the send in red or static error:

Screenshot 2024-11-12 at 12 39 01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants