On 30 Mar 2019, at 20:56, Denis Kudriashov wrote:

Hi Max.

сб, 30 мар. 2019 г. в 19:08, Max Leske <maxleske@gmail.com>:

...

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

Why it is important? If it is about speed then I don't believe you :) because your approach is very far from being optimized and I guess you are ok with that.

I know, and you're right of course. It's just the way I feel about the code :)

...

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

I also find that the last one resonates with what I would expect - and a real class autocompletes easily too. I also wonder if a class with subclasses should be considered abstract unless specifically marked otherwise (but we need a simple way of doing this if we can think of one - which isn’t overly clever) Tim Sent from my iPhone

On 30 Mar 2019, at 19:21, Cyril Ferlicot D. <cyril.ferlicot@gmail.com> wrote:

...

Le 30/03/2019 à 19:35, Denis Kudriashov a écrit : 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

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.

Hi,

Personally I always used the last one. (self = MyAbstractClass)

At first it is because it's the first implementation I saw. Then later, after seen the other two, I sticked with this one because it seems to be the simpler for me and I like my code to stay the simpler possible if there is no problem with that.

Until now I never got a problem with this implementation.

...

Best regards, Denis

-- Cyril Ferlicot https://ferlicot.fr

On 30 Mar 2019, at 21:05, Denis Kudriashov wrote:

We can also look into it from the other side. All these methods are definitely a kind of code duplication. I think what we need here is an explicit property for classes. It could be even part of class definition.

I'm not sure whether that's the right thing to do but I definitely like where you're going. BTW, class initialization via Monticello (class side #initialize) also doesn't use inheritance (although I've always considered that a bug). I.e., Monticello only evalutes #initialize on the class that installed the #initialize method and not on any subclass. Maybe a general mechanism for marking a class as abstract could also include a mechanism to "override" method lookup. Just some thoughts. Max

BTW do we plan new class definition in Pharo 8?

сб, 30 мар. 2019 г. в 18:35, Denis Kudriashov <dionisiydk@gmail.com>:

...

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

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

Am 30.03.2019 um 21:05 schrieb Denis Kudriashov <dionisiydk@gmail.com>:

We can also look into it from the other side. All these methods are definitely a kind of code duplication. I think what we need here is an explicit property for classes. It could be even part of class definition.

I use it myself but it always feels a bit strange. On the one hand a class is abstract if it includes something like subclassResponsibility and on the other hand we try to make it some kind of type. If it would be the latter one you could make that a Trait. Feels even more strange but .... Norbert

BTW do we plan new class definition in Pharo 8?

сб, 30 мар. 2019 г. в 18:35, Denis Kudriashov <dionisiydk@gmail.com>:

...

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

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

Calypso warns you about missing methods if it doesn’t understand a class is abstract, so it’s useful to avoid those warnings otherwise you become desensitised to them. Tim Sent from my iPhone

On 31 Mar 2019, at 22:06, ducasse <stepharo@netcourrier.com> wrote:

Frankly I found strange that we care about this because we can still instantiate an abstract class and I do not really see how annotating that a class is abstract help us. If I can instantiate an object and send messages and the class is abstract but there is no usage of abstract messages who cares? May be I do not have such scenarios.

Stef

...

On 31 Mar 2019, at 22:30, Norbert Hartl <norbert@hartl.name> wrote:

...

Am 30.03.2019 um 21:05 schrieb Denis Kudriashov <dionisiydk@gmail.com>:

We can also look into it from the other side. All these methods are definitely a kind of code duplication. I think what we need here is an explicit property for classes. It could be even part of class definition.

I use it myself but it always feels a bit strange. On the one hand a class is abstract if it includes something like subclassResponsibility and on the other hand we try to make it some kind of type. If it would be the latter one you could make that a Trait. Feels even more strange but ....

Norbert

...

BTW do we plan new class definition in Pharo 8?

сб, 30 мар. 2019 г. в 18:35, Denis Kudriashov <dionisiydk@gmail.com>:

...

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

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

This is a use case that is addressed at the level of the method, not of a class. I see the issue similarly to Stef: as I can utilize a class, it has little meaning to call it abstract. A missing method has a different meaning from the typical meaning associated with a class being abstract. Cheers, Doru

On Apr 1, 2019, at 10:54 AM, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:

Tim Mackinnon wrote

...

Calypso warns you about missing methods if it doesn’t understand a class is abstract, so it’s useful to avoid those warnings otherwise you become desensitised to them.

Tim

Or, if the class has subclasses, one could get a suggestion/action to implement the missing method as missingMethod ^self subclassResponsibility

Which also has the benefit of working nicely with the "expected (or was that "missing"?) protocol" functionality. Unless you meant something else?

Cheers, Henry

-- Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html

-- www.feenk.com "Problem solving should be focused on describing the problem in a way that makes the solution obvious."

On 1 Apr 2019, at 09:54, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:

Or, if the class has subclasses, one could get a suggestion/action to implement the missing method as missingMethod ^self subclassResponsibility

Which also has the benefit of working nicely with the "expected (or was that "missing"?) protocol" functionality. Unless you meant something else?

I don’t know if we are all talking about the same thing. In my image, I have a class where I haven’t got the isAbstract defined, and so Calypso shows a red message - as it thinks I’m supposed to implement #execute (if it knows my intent that my class is abstract, then it doesn’t show the red warning). I think this is Denis’ question? Sure, if I choose to use the abstract class, then its my choice, but as a lint checker -having useful warnings is helpful. But it is a nuisance defining isAbstract - and I’d almost prefer it assumes its abstract in this case (the normal expectations?) unless I tag it as concrete and signal to users that they might consider using my class (its probably a design smell these days though)

Hi, I think that it is good that isAbstract does not mean isInstantiatable. However, I also think that isAbstract is too generic and that it would be better to have the tools implement their meaning rather than relying on a generic term. Cheers, Doru

On Apr 23, 2019, at 5:58 PM, Andrei Chis <chisvasileandrei@gmail.com> wrote:

I ended up being surprised today that classes that have abstract methods (send #subclassResponsibility) were returning false to `isAbstract` and true to `hasAbstractMethods`. I see that the behavior for isAbstract changed over the years: when it was added [1] it was checking for abstract methods. Then it was changed to always return false and allow overrides [2].

Is there an explicit need to have/keep `isAbstract` at the class level? Users can still instantiate those classes, send message to them and get SubclassResponsibility errors. Tools could implement specific solutions for handling this.

Cheers, Andrei

[1] https://github.com/pharo-project/pharo/pull/541/files [2] https://github.com/pharo-project/pharo/pull/1090/files

On Mon, Apr 1, 2019 at 3:02 PM Tim Mackinnon <tim@testit.works> wrote:

...

On 1 Apr 2019, at 09:54, Henrik Sperre Johansen <henrik.s.johansen@veloxit.no> wrote:

Or, if the class has subclasses, one could get a suggestion/action to implement the missing method as missingMethod ^self subclassResponsibility

Which also has the benefit of working nicely with the "expected (or was that "missing"?) protocol" functionality. Unless you meant something else?

I don’t know if we are all talking about the same thing. In my image, I have a class where I haven’t got the isAbstract defined, and so Calypso shows a red message - as it thinks I’m supposed to implement #execute (if it knows my intent that my class is abstract, then it doesn’t show the red warning). I think this is Denis’ question?

Sure, if I choose to use the abstract class, then its my choice, but as a lint checker -having useful warnings is helpful. But it is a nuisance defining isAbstract - and I’d almost prefer it assumes its abstract in this case (the normal expectations?) unless I tag it as concrete and signal to users that they might consider using my class (its probably a design smell these days though)

-- www.feenk.com "If you interrupt the barber while he is cutting your hair, you will end up with a messy haircut."

On Mon, 29 Apr 2019 at 22:38, Guillermo Polito <guillermopolito@gmail.com> wrote:

A class is abstract by design and not necessarily because I can compute something from its implementation that tells me that maybe it is abstract. I somehow mark a class as abstract because it is abstract, not the other way around.

Randon idea... ``#Frame abstract`` returning a hypothetical instance of AbstractClassSymbol which double-dispatches to set a property of the class when it is created per... Object subclass: #Frame abstract instanceVariableNames: 'frameNumber frameRolls nextFrame' classVariableNames: '' package: 'Exercism-Bowling' cheers -ben