- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] how implement isAbstract?

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 <dionisiydk(a)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 > >

March 30, 2019

Re: [Pharo-dev] how implement isAbstract?

by Denis Kudriashov

Hi Max. ÑÐ±, 30 Ð¼Ð°Ñ. 2019 Ð³. Ð² 19:08, Max Leske <maxleske(a)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. > 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 > >

March 30, 2019

Re: [Pharo-dev] how implement isAbstract?

by 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. <cyril.ferlicot(a)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 >

March 30, 2019

Re: [Pharo-dev] how implement isAbstract?

by Cyril Ferlicot D.

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

March 30, 2019

Re: [Pharo-dev] how implement isAbstract?

by 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

March 30, 2019

how implement isAbstract?

by Denis Kudriashov

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

March 30, 2019

[Pharo 8.0] Build #205: 3033-TextMorphEditView-should-be-packaged-with-PluggableTextMorph

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #205 was: FAILURE. The Pull Request #3034 was integrated: "3033-TextMorphEditView-should-be-packaged-with-PluggableTextMorph" Pull request url: https://github.com/pharo-project/pharo/pull/3034 Issue Url: https://github.com/pharo-project/pharo/issues/3033 Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

March 30, 2019

[Pharo 8.0] Build #204: FAILURE

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #204 was: FAILURE. Could not extract further issue information from commit message: Update config.yml fix typo Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

March 30, 2019

Re: [Pharo-dev] Issue Tracker: enabled Projects (Boards)

by Marcus Denker

I added a description to the GitHub wiki: https://github.com/pharo-project/pharo/wiki/GitHub-Project-Boards > On 29 Mar 2019, at 09:52, Marcus Denker <marcus.denker(a)inria.fr> wrote: > > I moved them all to the Project level > > https://github.com/orgs/pharo-project/projects <https://github.com/orgs/pharo-project/projects> > > >>>> >>>>> On 18 Mar 2019, at 08:27, Marcus Denker <marcus.denker(a)inria.fr <mailto:marcus.denker@inria.fr>> wrote: >>>>> >>>>> Hi, >>>>> >>>>> We enabled the GitHub âProjectsâ feature for the pharo repo: >>>>> >>>>> https://github.com/pharo-project/pharo/projects <https://github.com/pharo-project/pharo/projects> >>>>> >>>>> >>>>> The idea is that one can add as many âKanbanâ style boards as one wants. >>>>> issues and pull requests can be added, as well as simple cards that are not yet issues. >>>>> >>>>> This is quite nice as an issue tracker gets very confusing and overwhelming as soon as there >>>>> are >500 issues. >>>>> >>>>> With these boards, we can create âviewsâ on this sea of issues. >>>>> >>>>> I created for now: >>>>> >>>>> -> some board related to specific parts of the system (e.g. Traits or Reflectivity) >>>>> example: https://github.com/pharo-project/pharo/projects/11 <https://github.com/pharo-project/pharo/projects/11> >>>>> -> "Pharo7 Backportingâ >>>>> https://github.com/pharo-project/pharo/projects/12 <https://github.com/pharo-project/pharo/projects/12> >>>>> -> "Simple Issues for Beginnersâ >>>>> https://github.com/pharo-project/pharo/projects/14 <https://github.com/pharo-project/pharo/projects/14> >>>>> Here the feature to be able to put things that are not yet issue tracker entries seems very useful >>>>> >>>>> >>>>> I think this can be quite niceâ¦ we should add more Projects as soon as we find that they make sense >>>>> and should move issues there. >>>>> >>>>> Marcus >>>> >>> >> >

March 30, 2019

[Pharo 8.0] Build #203: Integrating UndefinedClass, ClassDefinition and modified classBuilderâ¦

by ci-pharo-ci-jenkins2＠inria.fr

There is a new Pharo build available! The status of the build #203 was: FAILURE. The Pull Request #3030 was integrated: "Integrating UndefinedClass, ClassDefinition and modified classBuilderâ¦" Pull request url: https://github.com/pharo-project/pharo/pull/3030 Issue Url: https://github.com/pharo-project/pharo/issues/integratingUndefinedClass Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%2…

March 30, 2019