Hi Christoph,

On Sat, 21 Mar 2020, Thiede, Christoph wrote:

> 
> 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.

I agree about the names. I base my opinion on the fact that I never felt 
the need for such methods, though the pattern are fairly common. But, it's 
just my opinion.

> 
> 
> > (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?

Both #isNilOr: and #notNilAnd: return a boolean value. It is possible to 
use these methods and ignore their return values. E.g.:

object ifNotNil: [ collection add: object ].

can be written as:

object isNilOr: [ collection add: object ].

or as:

object notNilAnd: [ collection add: object ].


Levente

> 
> > 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!
> >
> >
> >
> >
> 
>