.all?{} and [].any?{} Behavior

Prev: [].all?{} and [].any?{} Behavior
Next: binary encode 7 ([7].pack("C")) as "\007" instead of "\a"

From: Rick DeNatale on 31 Jul 2010 12:22

---

On Sat, Jul 31, 2010 at 6:07 AM, Robert Klemme
<shortcutter(a)googlemail.com> wrote:
>
>> For your own usage as long as it doesn't mess up some other code you
>> are using, feel free.
>
> I disagree: IMHO it is a bad idea to change such fundamental behavior
> if only for own code.  This opens the door widely for all sorts of
> bugs and issues.  For example, you get used to #all? doing also the
> emptyness check and get confused when reading other code which of
> course relies on the regular behavior.  Or you forget the "require"
> for the file that changes semantics of #all? and #any? and receive in
> turn subtly bugs which might be hard to track down.  Even worse, you
> use library code that in turn uses #all? or #any? without you knowing
> it and this code suddenly breaks.

Whether or not it's a bad idea, and I tend to agree that it is. I
said what I did for at least two reasons:

1) I tend not to be puritanical, just as I wouldn't restrict what
anyone wanted to do in the privacy of their own homes unless it was
harmful to others, I think you should be able to write whatever code
you want to under the same philosophy, whether or not it's harmful to
you.

2) Since one of the most useful ways to learn anything in such a way
that you remember and internalize it is to make a mistake and realize
the consequences.
>
>> module Enumerable
>>  def non_vacuous_all?(&b)
>>    !empty? && all?(&b)
>>  end
>> end
>>
>> [3].all? {|element| element == 3 }  # => true
>> [3].all? {|element| element != 3 }  # => false
>>
>> [].all? {|element| element == 3 }   # => true
>> [].all? {|element| element != 3 }   # => true
>>
>> [3].non_vacuous_all? {|element| element == 3 }  # => true
>> [3].non_vacuous_all? {|element| element != 3 }  # => false
>>
>> [].non_vacuous_all? {|element| element == 3 }   # => false
>> [].non_vacuous_all? {|element| element != 3 }   # => false
>>
>> [].any? {|element| element == 3 }   # => false
>> [].any? {|element| element != 3 }   # => false
>>
>> There may be a better name than non_vacuous_all? but I can't think of one.
>
> I'd rather stick with two method calls because it makes crystall clear
> what's happening.  Also, you may first want to check for emptyness and
> if else branch based on that knowledge (or the other way round).  In
> other words: often you may want to separate both checks

Here I completely disagree. Extracting commonly used code to a well
named method is an essential part of writing and maintaining code.

for example, I see nothing wrong with the sum method which active
support adds to Enumerable

# Calculates a sum from the elements. Examples:
#
# payments.sum { |p| p.price * p.tax_rate }
# payments.sum(&:price)
#
# The latter is a shortcut for:
#
# payments.inject { |sum, p| sum + p.price }
#
# It can also calculate the sum without the use of a block.
#
# [5, 15, 10].sum # => 30
# ["foo", "bar"].sum # => "foobar"
# [[1, 2], [3, 1, 5]].sum => [1, 2, 3, 1, 5]
#
# The default sum of an empty list is zero. You can override this default:
#
# [].sum(Payment.new(0)) { |i| i.amount } # => Payment.new(0)
#
def sum(identity = 0, &block)
if block_given?
map(&block).sum(identity)
else
inject { |sum, element| sum + element } || identity
end
end

Following your argument, this is bad because you might want to use
inject and + separately.

But having such methods doesn't prevent you in the least from using
inject, +, empty?, any? or any other method used to implement a
slightly more abstract extracted method separately. It does help to
keep your code DRY and to make it more understandable overall since
you don't have to re-understand the effect of the separate invocations
each time you encounter them, as long as you are careful and name the
abstraction in an 'intention revealing' way.

And doing this also enables changing the implementation of the
abstraction without holistic changes to the code. Yes I know about de
Morgan's rules (I have a CS degree granted by a 1970's era Electrical
Engineering department). Placing the implementation in an abstraction
allows you to do the math proofs/unit testing and refactoring to meet
particular non-functional requirements in one place, which is a good
thing.

I recently was working on a refactoring a large Rails application
taken over from another development shop, which had several nasty bugs
on just this issue of all? returning true for an empty collection. It
turns out that there are definitely cases where you want to test that
a collection has at least 1 element and that all of the elements have
some property. Having said that perhaps a better name for the method
might be

at_least_one_and_all?

That might be a tad long, but I'd rather have a longer but more
intention revealing name, and let one of the several editors I use
deal with keeping my keystroke count down.

--
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Github: http://github.com/rubyredrick
Twitter: @RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

From: Robert Klemme on 1 Aug 2010 08:11

---

2010/7/31 Rick DeNatale <rick.denatale(a)gmail.com>:
> On Sat, Jul 31, 2010 at 6:07 AM, Robert Klemme
> <shortcutter(a)googlemail.com> wrote:
>>
>>> For your own usage as long as it doesn't mess up some other code you
>>> are using, feel free.
>>
>> I disagree: IMHO it is a bad idea to change such fundamental behavior
>> if only for own code.  This opens the door widely for all sorts of
>> bugs and issues.  For example, you get used to #all? doing also the
>> emptyness check and get confused when reading other code which of
>> course relies on the regular behavior.  Or you forget the "require"
>> for the file that changes semantics of #all? and #any? and receive in
>> turn subtly bugs which might be hard to track down.  Even worse, you
>> use library code that in turn uses #all? or #any? without you knowing
>> it and this code suddenly breaks.
>
> Whether or not it's a bad idea, and I tend to agree that it is.  I
> said what I did for at least two reasons:
>
> 1) I tend not to be puritanical, just as I wouldn't restrict what
> anyone wanted to do in the privacy of their own homes unless it was
> harmful to others, I think you should be able to write whatever code
> you want to under the same philosophy, whether or not it's harmful to
> you.

I did not want to sound puritanical, just point out that certain
habits are likely to have negative impact over time. And of course I
do agree that everyone should write their code as they see fit.

> 2) Since one of the most useful ways to learn anything in such a way
> that you remember and internalize it is to make a mistake and realize
> the consequences.

LOL

>>> module Enumerable
>>>  def non_vacuous_all?(&b)
>>>    !empty? && all?(&b)
>>>  end
>>> end
>>>
>>> [3].all? {|element| element == 3 }  # => true
>>> [3].all? {|element| element != 3 }  # => false
>>>
>>> [].all? {|element| element == 3 }   # => true
>>> [].all? {|element| element != 3 }   # => true
>>>
>>> [3].non_vacuous_all? {|element| element == 3 }  # => true
>>> [3].non_vacuous_all? {|element| element != 3 }  # => false
>>>
>>> [].non_vacuous_all? {|element| element == 3 }   # => false
>>> [].non_vacuous_all? {|element| element != 3 }   # => false
>>>
>>> [].any? {|element| element == 3 }   # => false
>>> [].any? {|element| element != 3 }   # => false
>>>
>>> There may be a better name than non_vacuous_all? but I can't think of one.
>>
>> I'd rather stick with two method calls because it makes crystall clear
>> what's happening.  Also, you may first want to check for emptyness and
>> if else branch based on that knowledge (or the other way round).  In
>> other words: often you may want to separate both checks
>
> Here I completely disagree.  Extracting commonly used code to a well
> named method is an essential part of writing and maintaining code.

While this is true as a general statement, creating too many methods
with trivial behavior can have a detrimental effect on software
readability and maintainability. The reason is that by doing this you
create additioanl artifacts that need to be documented and understand
by a reader. If on the other hand the idiom "!x.empty? && x.all?
{...}" is seen you can directly understand the behavior from three
basic mechanisms (#empty?, && and #all?) without having to resort to
other documentation. Finding a proper name for the combined method
also needs some careful consideration (as you have shown below).

> for example, I see nothing wrong with the sum method which active
> support adds to Enumerable
>
>  # Calculates a sum from the elements. Examples:
>  #
>  #  payments.sum { |p| p.price * p.tax_rate }
>  #  payments.sum(&:price)
>  #
>  # The latter is a shortcut for:
>  #
>  #  payments.inject { |sum, p| sum + p.price }
>  #
>  # It can also calculate the sum without the use of a block.
>  #
>  #  [5, 15, 10].sum # => 30
>  #  ["foo", "bar"].sum # => "foobar"
>  #  [[1, 2], [3, 1, 5]].sum => [1, 2, 3, 1, 5]
>  #
>  # The default sum of an empty list is zero. You can override this default:
>  #
>  #  [].sum(Payment.new(0)) { |i| i.amount } # => Payment.new(0)
>  #
>  def sum(identity = 0, &block)
>    if block_given?
>      map(&block).sum(identity)
>    else
>      inject { |sum, element| sum + element } || identity
>    end
>  end

There is at least the point that there is no universal neutral element
and 0 had to be chosen as default here; it's certainly the proper
value for many (most?) cases in practice but one should at least be
aware of this. If your collection contains strings you would need to
use "" as the neutral element (which strangely enough is called
"identity" here which to me sounds like the identity function which is
something different - but I'm nitpicking).

http://en.wikipedia.org/wiki/Neutral_element
http://en.wikipedia.org/wiki/Identity_function

> Following your argument, this is bad because you might want to use
> inject and + separately.

No, that was not my point. My point was that for the *same*
collection you may want to separate the emptyness check and the any /
all check that you combine in a single method. If you combine both in
a single method you cannot evalute results separately (or rather you
cannot use that method if you need the results separately). This is a
common case if you want to provide user responses that are easier to
understand for a human being.

> But having such methods doesn't prevent you in the least from using
> inject, +, empty?, any? or any other method used to implement a
> slightly more abstract extracted method separately.  It does help to
> keep your code DRY and to make it more understandable overall since
> you don't have to re-understand the effect of the separate invocations
> each time you encounter them, as long as you are careful and name the
> abstraction in an 'intention revealing' way.

As I said above, I do not think that creating new methods makes code
more understandable in all cases.

> And doing this also enables changing the implementation of the
> abstraction without holistic changes to the code.  Yes I know about de
> Morgan's rules (I have a CS degree granted by a 1970's era Electrical
> Engineering department).

The hint was intended for the OP. I never assumed you didn't know De
Morgan's rules.

> Placing the implementation in an abstraction
> allows you to do the math proofs/unit testing and refactoring to meet
> particular non-functional requirements in one place, which is a good
> thing.

Right.

> I recently was working on a refactoring a large Rails application
> taken over from another development shop, which had several nasty bugs
> on just this issue of all? returning true for an empty collection.  It
> turns out that there are definitely cases where you want to test that
> a collection has at least 1 element and that all of the elements have
> some property.  Having said that perhaps a better name for the method
> might be
>
>   at_least_one_and_all?
>
> That might be a tad long, but I'd rather have a longer but more
> intention revealing name, and let one of the several editors I use
> deal with keeping my keystroke count down.

I tend to prefer longer and more telling names as well. As you said,
modern environments provide plenty of utilities to deal with the nasty
typing gracefully.

Kind regards

robert

--
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

First  |  Prev  | 
Pages: 1 2
Prev: [].all?{} and [].any?{} Behavior
Next: binary encode 7 ([7].pack("C")) as "\007" instead of "\a"