[squeak-dev] #isNilOr:, #notNilAnd:

Thiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.de
Sat Mar 21 20:47:31 UTC 2020


Hi all,


I don't see why you would not like to see #isNilOr: or #notNilAnd: in "production" code? I think their names are self-explaining.

> (xyz := ...) notNil and: [xyz ...]

To me, this reads rather ugly. Assignment expressions are always a bit counter-intuitive as multiple things happen at a time. They're fine in order to optimize *really* performance-critical things, but I always try to avoid them in favor of overall readability.
Furthermore, I see a big advantage in avoiding temps wherever you can use block args: It helps you to minimize the scope of variables that are only useful to be accessed in a certain area. It just increases the defect potential if you have a not-very-short method like this:

someNotVeryShortMethod
| foo graffle bar baz |
foo := self someOperation.
graffle := foo asBarCompatibleThing.
bar := self someOtherOperation.
((baz := bar property) notNil and: [baz meetsConditionOne or: [baz meetsConditionTwo]])
    ifFalse: [^ foo].
^ foo mixWith: baz "Hm, do I need bar or baz here? Actually I meant bar, but that overcrowed temp list confused me so I just produced a bug"

Compared to this, a minimized scope can make things clearer:

someNotVeryShortMethod
| foo graffle bar |
foo := self someOperation.
graffle := foo convertForBarCompatibility.
bar := self someOtherOperation.
(bar property notNilAnd: [:baz | meetsConditionOne or: [baz meetsConditionTwo]])
    ifFalse: [^ foo].
^ foo mixWith: bar "Ah, baz is not known as this place and I got a compiler error. So this must be bar!"

Some other thing I see quite often is the following:

checkArg: arg
^ arg simpleProperty notNil and: [
    self basicCheckArg: arg simpleProperty]

While this is fine for most cases, this makes it harder to implement #simpleProperty as an expensive or non-side-effect-free operation. Thus it would be better to write the following instead:

checkArg: arg
property := arg simpleProperty.
^ property notNil and: [
    self basicCheckArg: property]

But programmers are lazy, so they often don't introduce an extra tempvar ... Instead, any protocol that supplies arguments to the caller encourages you to use these arguments:

checkArg: arg
^ arg simpleProperty notNilAnd: [:property |
    self basicCheckArg: property]

Plus, it's just shorter :-)

> They are especially susceptible to misuse when their return value is ignored; in which case they are both equivalent to #ifNotNil:.

What return value are you talking about? Did you mean the block argument?

> Have you had a look at Java 8's Optional class? Nil can receive messages in Smalltalk, which Java null can't. Is nil therefore a good empty Optional or should these concepts be separate?

Are you talking about some kind of automatic null object that ignores every message? Or do you complain that "nil handles: #foo" does not raise an error, just for example? :-)

Best,
Christoph

________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Levente Uzonyi <leves at caesar.elte.hu>
Gesendet: Samstag, 21. März 2020 20:25 Uhr
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] #isNilOr:, #notNilAnd:

Hi Christoph,

They are okay for scripts, but we know that methods like that always end
up being used out of scripts, and are even misused every now and then.
They are especially susceptible to misuse when their return value is
ignored; in which case they are both equivalent to #ifNotNil:.

I'm also not sure whether these methods belong to ProtoObject or not.
I know the other nil handling methods are there as well, but that doesn't
mean all such methods belong there.
E.g. #ifNil:/#ifNotNil: should be there because the current compiler and
VM will make those available for all objects provided the methods are
inlined. But #isNil and #notNil have no such properties, so they belong
more to Object than ProtoObject in my opinion.


Levente

On Mon, 16 Mar 2020, Thiede, Christoph wrote:

>
> Hi all,
>
>
> after the release has been managed successfully, I would like to kindly push this proposal. In the latest months, I found myself to really often desiring #isNilOr: or #notNilAnd:. How would you think about these convenience
> selectors? :-)
>
>
> Best,
>
> Christoph
>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Thiede, Christoph
> Gesendet: Mittwoch, 11. Dezember 2019 11:01:44
> An: Squeak Dev
> Betreff: #isNilOr:, #notNilAnd:
>
> Hi all, just another idea for a hypothetic extension to ProtoObject:
>
>
> ProtoObject >> #isNilOr: aBlock
>
> ^ aBlock cull: self
>
> UndefinedObject >> #isNilOr: aBlock
>
> ^ true
>
> ProtoObject >> #notNilAnd: aBlock
>
> ^ aBlock cull: self
>
> UndefinedObject >> #notNilAnd: aBlock
>
> ^ false
>
>
> Examples:
>
> parentObject notNilAnd: #canBrowseSubtopic.
>
> helpClass notNilAnd: [:hC| hC usesCodeStyling].
>
> self selectionIndex isNilOr: [:index | index > 2].
>
> anObject isNilOr: #isZero.
>
> foo isNilOr: #isNotEmpty.
>
>
> Just a bit syntactic sugar for idiomatic problems such as below. In particular, I found several possible applications for #notNilAnd: in my image.
>
>
> Go ahead and tell me why it would be a bad idea :)
>
>
> Best,
>
> Christoph
>
>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Thiede, Christoph
> Gesendet: Freitag, 25. Oktober 2019 19:21 Uhr
> An: gettimothy via Squeak-dev
> Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.123.mcz
>
> If you're needing a decision, I'd vote for option 3.
>
> _________________________________________________________________________________________________________________________________________________________________________________________________________________________________
> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
> Gesendet: Mittwoch, 16. Oktober 2019 11:14:15
> An: gettimothy via Squeak-dev
> Betreff: Re: [squeak-dev] The Inbox: HelpSystem-Core-ct.123.mcz
> Quick suggestion on the formatting. This one:
> ^ self canBrowseTopic or: [
>    parentTopic ifNotNil: #canBrowseSubtopic ifNil: [false]]
>
> could become:
>
> ^ self canBrowseTopic or: [
>    parentTopic
>    ifNil: [false]
>    ifNotNil: [:topic | topic canBrowseSubtopic]]
>
> or:
>
> ^ self canBrowseTopic
>       or: [parentTopic notNil and: [parentTopic canBrowseSubtopic]]
>
> Hmmm... what are other opinions on this? There is no need for #ifNil/ifNotNil in such a boolean expression?
>
> Best,
> Marcel
>
>
>       Am 13.10.2019 21:04:19 schrieb commits at source.squeak.org <commits at source.squeak.org>:
>
>       A new version of HelpSystem-Core was added to project The Inbox:
>       http://source.squeak.org/inbox/HelpSystem-Core-ct.123.mcz
>
>       ==================== Summary ====================
>
>       Name: HelpSystem-Core-ct.123
>       Author: ct
>       Time: 13 October 2019, 9:04:08.373932 pm
>       UUID: dec7ceca-320f-d945-8d2a-c2f6a5e49a52
>       Ancestors: HelpSystem-Core-ct.120
>
>       Refactors HelpBrowser menu: Move menu stuff from HelpBrowser into HelpTopic hierarchy in favor of a better object design
>
>       Thanks again, Marcel :-)
>
>       =============== Diff against HelpSystem-Core-ct.120 ===============
>
>       Item was added:
>       + ----- Method: AbstractHelpTopic>>browseTopicFromParent: (in category 'tools') -----
>       + browseTopicFromParent: parentTopic
>       +
>       + self canBrowseTopic
>       + ifTrue: [^ self browseTopic].
>       + parentTopic canBrowseSubtopic
>       + ifTrue: [^ parentTopic browseSubtopic: self].
>       + !
>
>       Item was added:
>       + ----- Method: AbstractHelpTopic>>canBrowseSubtopic (in category 'testing') -----
>       + canBrowseSubtopic
>       +
>       + ^ false!
>
>       Item was added:
>       + ----- Method: AbstractHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ false!
>
>       Item was added:
>       + ----- Method: AbstractHelpTopic>>canBrowseTopicFromParent: (in category 'testing') -----
>       + canBrowseTopicFromParent: parentTopic
>       +
>       + ^ self canBrowseTopic or: [
>       + parentTopic ifNotNil: #canBrowseSubtopic ifNil: [false]]!
>
>       Item was added:
>       + ----- Method: AbstractHelpTopic>>topicMenu:parentTopic: (in category 'menus') -----
>       + topicMenu: aMenu parentTopic: parentTopic
>       +
>       + aMenu
>       + add: 'Inspect (i)' translated target: self action: #inspect;
>       + add: 'Explore (I)' translated target: self action: #explore.
>       + (self canBrowseTopicFromParent: parentTopic)
>       + ifTrue: [
>       + aMenu add: 'Browse (b)' translated
>       + target: self
>       + selector: #browseTopicFromParent:
>       + argumentList: {parentTopic} ].
>       +
>       + ^ aMenu!
>
>       Item was added:
>       + ----- Method: AbstractHelpTopic>>topicMenuKey:fromParent: (in category 'menus') -----
>       + topicMenuKey: aChar fromParent: parentTopic
>       +
>       + aChar
>       + caseOf: {
>       + [$b] -> [(self canBrowseTopicFromParent: parentTopic)
>       + ifTrue: [ self browseTopicFromParent: parentTopic ]].
>       + [$i] -> [self inspect].
>       + [$I] -> [self explore] }
>       + otherwise: [^ false].
>       + ^ true!
>
>       Item was added:
>       + ----- Method: ClassAPIHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ true!
>
>       Item was added:
>       + ----- Method: ClassBasedHelpTopic>>canBrowseSubtopic (in category 'testing') -----
>       + canBrowseSubtopic
>       +
>       + ^ true!
>
>       Item was added:
>       + ----- Method: ClassBasedHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ true!
>
>       Item was added:
>       + ----- Method: DirectoryBasedHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ true!
>
>       Item was added:
>       + ----- Method: FileBasedHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ true!
>
>       Item was removed:
>       - ----- Method: HelpBrowser>>browseTopic (in category 'actions') -----
>       - browseTopic
>       -
>       - ^ (self currentTopic respondsTo: #browseTopic)
>       - ifTrue: [self currentTopic browseTopic]
>       - ifFalse: [self currentParentTopic browseSubtopic: self currentTopic]!
>
>       Item was removed:
>       - ----- Method: HelpBrowser>>canBrowseTopic (in category 'testing') -----
>       - canBrowseTopic
>       -
>       - ^ (self currentTopic respondsTo: #browseTopic)
>       - or: [self currentParentTopic respondsTo: #browseSubtopic:]!
>
>       Item was removed:
>       - ----- Method: HelpBrowser>>exploreTopic (in category 'actions') -----
>       - exploreTopic
>       -
>       - ^ self currentTopic explore!
>
>       Item was removed:
>       - ----- Method: HelpBrowser>>inspectTopic (in category 'actions') -----
>       - inspectTopic
>       -
>       - ^ self currentTopic inspect!
>
>       Item was changed:
>       ----- Method: HelpBrowser>>treeKey:from:event: (in category 'menus') -----
>       treeKey: aChar from: aView event: anEvent
>
>       anEvent anyModifierKeyPressed ifFalse: [^ false].
>       + ^ (self currentTopic topicMenuKey: aChar fromParent: self currentParentTopic)!
>       - aChar
>       - caseOf: {
>       - [$b] -> [self browseTopic].
>       - [$i] -> [self inspectTopic].
>       - [$I] -> [self exploreTopic]. }
>       - otherwise: [^ false].
>       - ^ true!
>
>       Item was changed:
>       ----- Method: HelpBrowser>>treeListMenu: (in category 'menus') -----
>       treeListMenu: aMenu
>
>
>       + ^ self currentTopic
>       + ifNil: [aMenu]
>       + ifNotNil: [:topic | topic
>       + topicMenu: aMenu
>       + parentTopic: self currentParentTopic]!
>       - self currentTopic ifNil: [^ aMenu].
>       -
>       - aMenu
>       - add: 'Inspect (i)' action: #inspectTopic;
>       - add: 'Explore (I)' action: #exploreTopic.
>       -
>       - self canBrowseTopic ifTrue: [
>       - aMenu
>       - addLine;
>       - add: 'Browse (b)' action: #browseTopic].
>       -
>       - ^ aMenu!
>
>       Item was added:
>       + ----- Method: MethodListHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ true!
>
>       Item was added:
>       + ----- Method: PackageAPIHelpTopic>>canBrowseTopic (in category 'testing') -----
>       + canBrowseTopic
>       +
>       + ^ true!
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20200321/a52a4c8c/attachment-0001.html>


More information about the Squeak-dev mailing list