pharo-graphics / Roassal

The Roassal Visualization Engine
MIT License
16 stars 12 forks source link

Remove explicitRequirement #80

Open JanBliznicenko opened 2 weeks ago

JanBliznicenko commented 2 weeks ago

explicitRequirement is deprecated in Pharo 13 because of being cause of slowdowns and bugs due to its complexity: https://github.com/pharo-project/pharo/pull/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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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