how implement isAbstract?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

how implement isAbstract?

Denis Kudriashov
Hello.

We did recently several cleanups by marking abstract classes as abstract using #isAbstract method (https://github.com/pharo-project/pharo/pull/3087https://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

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Max Leske

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 | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

CyrilFerlicot
In reply to this post by Denis Kudriashov
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/3087https://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


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Tim Mackinnon
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. <[hidden email]> 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
>


Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Denis Kudriashov
In reply to this post by Max Leske
Hi Max.

сб, 30 мар. 2019 г. в 19:08, Max Leske <[hidden email]>:

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. 

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 | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Denis Kudriashov
In reply to this post by Denis Kudriashov
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.
BTW do we plan new class definition in Pharo 8? 

сб, 30 мар. 2019 г. в 18:35, Denis Kudriashov <[hidden email]>:
Hello.

We did recently several cleanups by marking abstract classes as abstract using #isAbstract method (https://github.com/pharo-project/pharo/pull/3087https://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

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Max Leske
In reply to this post by Denis Kudriashov
On 30 Mar 2019, at 20:56, Denis Kudriashov wrote:

> Hi Max.
>
> сб, 30 мар. 2019 г. в 19:08, Max Leske <[hidden email]>:
>
>> 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
>>
>>



Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Max Leske
In reply to this post by Denis Kudriashov
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
> <[hidden email]>:
>
>> 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
>>
>>



Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

ducasse
In reply to this post by Denis Kudriashov


> On 30 Mar 2019, at 21:05, Denis Kudriashov <[hidden email]> 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.
> BTW do we plan new class definition in Pharo 8?

so far not.
Just a better infrastructure.
I also think that this abstract point is not really exciting.

Stef

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

NorbertHartl
In reply to this post by Denis Kudriashov


Am 30.03.2019 um 21:05 schrieb Denis Kudriashov <[hidden email]>:

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 <[hidden email]>:
Hello.

We did recently several cleanups by marking abstract classes as abstract using #isAbstract method (https://github.com/pharo-project/pharo/pull/3087https://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

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

ducasse
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 <[hidden email]> wrote:



Am 30.03.2019 um 21:05 schrieb Denis Kudriashov <[hidden email]>:

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 <[hidden email]>:
Hello.

We did recently several cleanups by marking abstract classes as abstract using #isAbstract method (https://github.com/pharo-project/pharo/pull/3087https://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


Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Tim Mackinnon
 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 <[hidden email]> 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 <[hidden email]> wrote:



Am 30.03.2019 um 21:05 schrieb Denis Kudriashov <[hidden email]>:

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 <[hidden email]>:
Hello.

We did recently several cleanups by marking abstract classes as abstract using #isAbstract method (https://github.com/pharo-project/pharo/pull/3087https://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


Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Henrik Sperre Johansen
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

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Tudor Girba-2
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 <[hidden email]> 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."







Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Henrik Sperre Johansen
I guess I could have added additional constraints
"and the missing method is called from a method on the "abstract class", and
the missing method is defined on subclasses", but I thought that implicit
from the context :)

To me, an abstract class is one you either can't instatiate (through
overriding new), or one containing methods which delegate to methods
implemented by subclasses, so it makes no sense to use instances of the
class itself.
The use case is addressed at the method level, yes, but it *is* for the
typical meaning associated with a class being abstract, at least to me.

Cheers,
Henry


Tudor Girba-2 wrote

> 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 &lt;

> henrik.s.johansen@

> &gt; 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."





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

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Tim Mackinnon
In reply to this post by Henrik Sperre Johansen
On 1 Apr 2019, at 09:54, Henrik Sperre Johansen <[hidden email]> 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)

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Andrei Chis
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 <[hidden email]> wrote:

>
> On 1 Apr 2019, at 09:54, Henrik Sperre Johansen <[hidden email]> 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)
>

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Tudor Girba-2
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 <[hidden email]> 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 <[hidden email]> wrote:
>>
>> On 1 Apr 2019, at 09:54, Henrik Sperre Johansen <[hidden email]> 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."


Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Guillermo Polito
Hi Andrei, Doru,

Sorry for the late reply, I've been away on vacations last week :).

Denis introduced originally the behaviour making classes return true for #isAbstract only when they had #subclassResponsibility methods in them.
Originally, if I'm not mistaken, it was to introduce the `should be implemented` protocol in Calypso: this was originally shown for subclasses of abstract classes that were not abstract themselves (under that definition).

image.png

My main point against that definition is that sometimes I have hierarchies like:

A <|- B <|- C

Where conceptually both A and B are abstract, but I only put the subclass responsibility methods in A.
However under that definition B was not considered as abstract, though in my design it was.
And then I had Calypso asking me all the time to <<implement>> the methods in the superclass, which I did not want to!

My point at the time (and that resulted in the split of isAbstract into hasAbstractMethods) was that (at least for me) the heuristic of having abstract methods was super weak.
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.

On Tue, Apr 23, 2019 at 6:10 PM Tudor Girba <[hidden email]> wrote:
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 <[hidden email]> 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 <[hidden email]> wrote:
>>
>> On 1 Apr 2019, at 09:54, Henrik Sperre Johansen <[hidden email]> 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."




--

   

Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - http://www.cnrs.fr


Web: http://guillep.github.io

Phone: +33 06 52 70 66 13

Reply | Threaded
Open this post in threaded view
|

Re: how implement isAbstract?

Ben Coman


On Mon, 29 Apr 2019 at 22:38, Guillermo Polito <[hidden email]> 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