Collection>>ifNotEmpty: changed semantics

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

Collection>>ifNotEmpty: changed semantics

Simon Denier-3
I just noticed that the semantic of Collection>>ifNotEmpty: has changed in subtle ways between Pharo 1.0 and 1.1

In 1.0
Collection>>ifNotEmpty: aBlock
        "Evaluate the given block unless the receiver is empty.

     If the block has an argument, eval with the receiver as its argument,
     but it might be better to use ifNotEmptyDo: to make the code easier to
     understand"

        ^self isEmpty ifFalse: [aBlock valueWithPossibleArgument: self].

-----> it returns nil if collection is empty


in 1.1
Collection>>ifNotEmpty: aBlock
        "Evaluate the given block unless the receiver is empty.

     If the block has an argument, eval with the receiver as its argument,
     but it might be better to use ifNotEmptyDo: to make the code easier to
     understand"

        self isEmpty ifFalse: [^ aBlock valueWithPossibleArgument: self].

----> the return is inside the block, now it returns the collection if collection is empty


Apparently all similar code (#ifEmpty:....) has been adapted in the same way.

Of course I would not have stumbled upon this change if it didn't break some of my code which rely on the previous assumption.

Now I would like to understand before making some change in my code. Is it considered bad practice to return nil in such cases? What is the rationale to return yourself instead of nil?

--
Simon



_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Collection>>ifNotEmpty: changed semantics

Nicolas Cellier
2010/7/2 Simon Denier <[hidden email]>:

> I just noticed that the semantic of Collection>>ifNotEmpty: has changed in subtle ways between Pharo 1.0 and 1.1
>
> In 1.0
> Collection>>ifNotEmpty: aBlock
>        "Evaluate the given block unless the receiver is empty.
>
>     If the block has an argument, eval with the receiver as its argument,
>     but it might be better to use ifNotEmptyDo: to make the code easier to
>     understand"
>
>        ^self isEmpty ifFalse: [aBlock valueWithPossibleArgument: self].
>
> -----> it returns nil if collection is empty
>
>
> in 1.1
> Collection>>ifNotEmpty: aBlock
>        "Evaluate the given block unless the receiver is empty.
>
>     If the block has an argument, eval with the receiver as its argument,
>     but it might be better to use ifNotEmptyDo: to make the code easier to
>     understand"
>
>        self isEmpty ifFalse: [^ aBlock valueWithPossibleArgument: self].
>
> ----> the return is inside the block, now it returns the collection if collection is empty
>
>
> Apparently all similar code (#ifEmpty:....) has been adapted in the same way.
>
> Of course I would not have stumbled upon this change if it didn't break some of my code which rely on the previous assumption.
>
> Now I would like to understand before making some change in my code. Is it considered bad practice to return nil in such cases? What is the rationale to return yourself instead of nil?
>

The new version is better because:
- it is homogeneous with ifNil: behaviour
- it enables writing:  self classifyMethodAs: (myProtocol ifEmpty:
['As yet unclassified'])

Nicolas

> --
> Simon
>
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Collection>>ifNotEmpty: changed semantics

Nicolas Cellier
2010/7/2 Nicolas Cellier <[hidden email]>:

> 2010/7/2 Simon Denier <[hidden email]>:
>> I just noticed that the semantic of Collection>>ifNotEmpty: has changed in subtle ways between Pharo 1.0 and 1.1
>>
>> In 1.0
>> Collection>>ifNotEmpty: aBlock
>>        "Evaluate the given block unless the receiver is empty.
>>
>>     If the block has an argument, eval with the receiver as its argument,
>>     but it might be better to use ifNotEmptyDo: to make the code easier to
>>     understand"
>>
>>        ^self isEmpty ifFalse: [aBlock valueWithPossibleArgument: self].
>>
>> -----> it returns nil if collection is empty
>>
>>
>> in 1.1
>> Collection>>ifNotEmpty: aBlock
>>        "Evaluate the given block unless the receiver is empty.
>>
>>     If the block has an argument, eval with the receiver as its argument,
>>     but it might be better to use ifNotEmptyDo: to make the code easier to
>>     understand"
>>
>>        self isEmpty ifFalse: [^ aBlock valueWithPossibleArgument: self].
>>
>> ----> the return is inside the block, now it returns the collection if collection is empty
>>
>>
>> Apparently all similar code (#ifEmpty:....) has been adapted in the same way.
>>
>> Of course I would not have stumbled upon this change if it didn't break some of my code which rely on the previous assumption.
>>
>> Now I would like to understand before making some change in my code. Is it considered bad practice to return nil in such cases? What is the rationale to return yourself instead of nil?
>>
>
> The new version is better because:
> - it is homogeneous with ifNil: behaviour
> - it enables writing:  self classifyMethodAs: (myProtocol ifEmpty:
> ['As yet unclassified'])
>
> Nicolas
>

Oops, embarassing, I didn't answer the good question :(
Maybe it's homogeneous with ifNotNil: ?

Nicolas

>> --
>> Simon
>>
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Collection>>ifNotEmpty: changed semantics

Eliot Miranda-2
In reply to this post by Nicolas Cellier
Hi Nicolas,

    in any case the comment needs to specify that behavior, e.g.

Collection>>ifNotEmpty: aBlock
     "Evaluate the given block with the receiver as argument, answering its value
      unless the receiver is empty, in which case answer the receiver."
    ^self isEmpty
          ifTrue: [self]
          ifFalse: [aBlock valueWithPossibleArgument: self]

and in this case IMO the explicit return self is better than the implicit one.

On Fri, Jul 2, 2010 at 2:34 PM, Nicolas Cellier <[hidden email]> wrote:
2010/7/2 Simon Denier <[hidden email]>:
> I just noticed that the semantic of Collection>>ifNotEmpty: has changed in subtle ways between Pharo 1.0 and 1.1
>
> In 1.0
> Collection>>ifNotEmpty: aBlock
>        "Evaluate the given block unless the receiver is empty.
>
>     If the block has an argument, eval with the receiver as its argument,
>     but it might be better to use ifNotEmptyDo: to make the code easier to
>     understand"
>
>        ^self isEmpty ifFalse: [aBlock valueWithPossibleArgument: self].
>
> -----> it returns nil if collection is empty
>
>
> in 1.1
> Collection>>ifNotEmpty: aBlock
>        "Evaluate the given block unless the receiver is empty.
>
>     If the block has an argument, eval with the receiver as its argument,
>     but it might be better to use ifNotEmptyDo: to make the code easier to
>     understand"
>
>        self isEmpty ifFalse: [^ aBlock valueWithPossibleArgument: self].
>
> ----> the return is inside the block, now it returns the collection if collection is empty
>
>
> Apparently all similar code (#ifEmpty:....) has been adapted in the same way.
>
> Of course I would not have stumbled upon this change if it didn't break some of my code which rely on the previous assumption.
>
> Now I would like to understand before making some change in my code. Is it considered bad practice to return nil in such cases? What is the rationale to return yourself instead of nil?
>

The new version is better because:
- it is homogeneous with ifNil: behaviour
- it enables writing:  self classifyMethodAs: (myProtocol ifEmpty:
['As yet unclassified'])

Nicolas

> --
> Simon
>
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Collection>>ifNotEmpty: changed semantics

Nicolas Cellier
100% agreement on this

2010/7/3 Eliot Miranda <[hidden email]>:

> Hi Nicolas,
>     in any case the comment needs to specify that behavior, e.g.
> Collection>>ifNotEmpty: aBlock
>      "Evaluate the given block with the receiver as argument, answering its
> value
>       unless the receiver is empty, in which case answer the receiver."
>     ^self isEmpty
>           ifTrue: [self]
>           ifFalse: [aBlock valueWithPossibleArgument: self]
>
> and in this case IMO the explicit return self is better than the implicit
> one.
> On Fri, Jul 2, 2010 at 2:34 PM, Nicolas Cellier
> <[hidden email]> wrote:
>>
>> 2010/7/2 Simon Denier <[hidden email]>:
>> > I just noticed that the semantic of Collection>>ifNotEmpty: has changed
>> > in subtle ways between Pharo 1.0 and 1.1
>> >
>> > In 1.0
>> > Collection>>ifNotEmpty: aBlock
>> >        "Evaluate the given block unless the receiver is empty.
>> >
>> >     If the block has an argument, eval with the receiver as its
>> > argument,
>> >     but it might be better to use ifNotEmptyDo: to make the code easier
>> > to
>> >     understand"
>> >
>> >        ^self isEmpty ifFalse: [aBlock valueWithPossibleArgument: self].
>> >
>> > -----> it returns nil if collection is empty
>> >
>> >
>> > in 1.1
>> > Collection>>ifNotEmpty: aBlock
>> >        "Evaluate the given block unless the receiver is empty.
>> >
>> >     If the block has an argument, eval with the receiver as its
>> > argument,
>> >     but it might be better to use ifNotEmptyDo: to make the code easier
>> > to
>> >     understand"
>> >
>> >        self isEmpty ifFalse: [^ aBlock valueWithPossibleArgument: self].
>> >
>> > ----> the return is inside the block, now it returns the collection if
>> > collection is empty
>> >
>> >
>> > Apparently all similar code (#ifEmpty:....) has been adapted in the same
>> > way.
>> >
>> > Of course I would not have stumbled upon this change if it didn't break
>> > some of my code which rely on the previous assumption.
>> >
>> > Now I would like to understand before making some change in my code. Is
>> > it considered bad practice to return nil in such cases? What is the
>> > rationale to return yourself instead of nil?
>> >
>>
>> The new version is better because:
>> - it is homogeneous with ifNil: behaviour
>> - it enables writing:  self classifyMethodAs: (myProtocol ifEmpty:
>> ['As yet unclassified'])
>>
>> Nicolas
>>
>> > --
>> > Simon
>> >
>> >
>> >
>> > _______________________________________________
>> > Pharo-project mailing list
>> > [hidden email]
>> > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>> >
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Collection>>ifNotEmpty: changed semantics

Stéphane Ducasse
thanks for the discussion!
http://code.google.com/p/pharo/issues/detail?id=2636

Stef



On Jul 3, 2010, at 9:04 AM, Nicolas Cellier wrote:

> 100% agreement on this
>
> 2010/7/3 Eliot Miranda <[hidden email]>:
>> Hi Nicolas,
>>     in any case the comment needs to specify that behavior, e.g.
>> Collection>>ifNotEmpty: aBlock
>>      "Evaluate the given block with the receiver as argument, answering its
>> value
>>       unless the receiver is empty, in which case answer the receiver."
>>     ^self isEmpty
>>           ifTrue: [self]
>>           ifFalse: [aBlock valueWithPossibleArgument: self]
>>
>> and in this case IMO the explicit return self is better than the implicit
>> one.
>> On Fri, Jul 2, 2010 at 2:34 PM, Nicolas Cellier
>> <[hidden email]> wrote:
>>>
>>> 2010/7/2 Simon Denier <[hidden email]>:
>>>> I just noticed that the semantic of Collection>>ifNotEmpty: has changed
>>>> in subtle ways between Pharo 1.0 and 1.1
>>>>
>>>> In 1.0
>>>> Collection>>ifNotEmpty: aBlock
>>>>        "Evaluate the given block unless the receiver is empty.
>>>>
>>>>     If the block has an argument, eval with the receiver as its
>>>> argument,
>>>>     but it might be better to use ifNotEmptyDo: to make the code easier
>>>> to
>>>>     understand"
>>>>
>>>>        ^self isEmpty ifFalse: [aBlock valueWithPossibleArgument: self].
>>>>
>>>> -----> it returns nil if collection is empty
>>>>
>>>>
>>>> in 1.1
>>>> Collection>>ifNotEmpty: aBlock
>>>>        "Evaluate the given block unless the receiver is empty.
>>>>
>>>>     If the block has an argument, eval with the receiver as its
>>>> argument,
>>>>     but it might be better to use ifNotEmptyDo: to make the code easier
>>>> to
>>>>     understand"
>>>>
>>>>        self isEmpty ifFalse: [^ aBlock valueWithPossibleArgument: self].
>>>>
>>>> ----> the return is inside the block, now it returns the collection if
>>>> collection is empty
>>>>
>>>>
>>>> Apparently all similar code (#ifEmpty:....) has been adapted in the same
>>>> way.
>>>>
>>>> Of course I would not have stumbled upon this change if it didn't break
>>>> some of my code which rely on the previous assumption.
>>>>
>>>> Now I would like to understand before making some change in my code. Is
>>>> it considered bad practice to return nil in such cases? What is the
>>>> rationale to return yourself instead of nil?
>>>>
>>>
>>> The new version is better because:
>>> - it is homogeneous with ifNil: behaviour
>>> - it enables writing:  self classifyMethodAs: (myProtocol ifEmpty:
>>> ['As yet unclassified'])
>>>
>>> Nicolas
>>>
>>>> --
>>>> Simon
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Pharo-project mailing list
>>>> [hidden email]
>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>>
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project