Hi Denis,

I'm not too happy with any of those. For my own applications I use the following:

isAbstract
        ^ self subclasses notEmpty and: [
                (self class localSelectors includes: #isReallyAbstract) not or: 
[
                        self isReallyAbstract ] ]


isReallyAbstract
        "Override this method in any class that has children but should
        *not* be considered abstract (while the children may be)."
        ^ true

The reason, as stated in the method comment, is that in nearly all of my cases a class with subclasses should be considered abstract. Now, I don't want to go and implement #isAbstract on each of those classes but at the same time I need an escape hatch *which is not inherited*. That's why I work around method lookup by checking for the existence of the method first.

I don't like how I have to hack around the method lookup but this works far better from me than the class equality / identity check.


On 30 Mar 2019, at 19:35, Denis Kudriashov wrote:

Hello.

We did recently several cleanups by marking abstract classes as abstract using #isAbstract method (https://github.com/pharo-project/pharo/pull/3087,
https://github.com/pharo-ide/Calypso/pull/462) .

I would like to discuss here and decide what the right way to implement
this method.

The logic behind is to only return true when receiver is defining class of
this method. And it should be false for any subclasses (if they do not
implement own #isAbstract method).

There is old pattern for implementation to compare name of class:

MyAbstractClass class>>isAbstract
      ^self name == #MyAbstractClass


It is used in many places (mostly tests). And it was used in recent Pharo
PR (3087).

We have another pattern in other places which simply compare self with
class:

MyAbstractClass class>>isAbstract
      ^self == MyAbstractClass

Otherwise, this would be my preferred method:
1. minimum number of byte codes
2. Simple for refactoring tools to update the binding
3. no use of #name



Cheers,
Max



And in Calypso I used "more simplified" version using equality:

MyAbstractClass class>>isAbstract
      ^self = MyAbstractClass

I think we would all agree that simplest version is last one. It does not
raise any question about why we compare name or why we use identity
comparison. So this is my choice in this discussion.

Please write arguments about your preferred implementation. And let's
choose single way at the end. There is an idea to add a command into
browser to make classes abstract. And this decision will be used as a
template.


Best regards,
Denis


Reply via email to