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

2010/7/31 Rick DeNatale <[e-mail]>:
> On Sat, Jul 31, 2010 at 6:07 AM, Robert Klemme
> <[e-mail]> 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. =A0This opens the door widely for all sorts of
>> bugs and issues. =A0For 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. =A0Or 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. =A0Even 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. =A0I
> 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
>>> =A0def non_vacuous_all?(&b)
>>> =A0 =A0!empty? && all?(&b)
>>> =A0end
>>> end
>>>
>>> [3].all? {|element| element =3D=3D 3 } =A0# =3D> true
>>> [3].all? {|element| element !=3D 3 } =A0# =3D> false
>>>
>>> [].all? {|element| element =3D=3D 3 } =A0 # =3D> true
>>> [].all? {|element| element !=3D 3 } =A0 # =3D> true
>>>
>>> [3].non_vacuous_all? {|element| element =3D=3D 3 } =A0# =3D> true
>>> [3].non_vacuous_all? {|element| element !=3D 3 } =A0# =3D> false
>>>
>>> [].non_vacuous_all? {|element| element =3D=3D 3 } =A0 # =3D> false
>>> [].non_vacuous_all? {|element| element !=3D 3 } =A0 # =3D> false
>>>
>>> [].any? {|element| element =3D=3D 3 } =A0 # =3D> false
>>> [].any? {|element| element !=3D 3 } =A0 # =3D> false
>>>
>>> There may be a better name than non_vacuous_all? but I can't think of o=
ne.
>>
>> I'd rather stick with two method calls because it makes crystall clear
>> what's happening. =A0Also, you may first want to check for emptyness and
>> if else branch based on that knowledge (or the other way round). =A0In
>> other words: often you may want to separate both checks
>
> Here I completely disagree. =A0Extracting 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
>
> =A0# Calculates a sum from the elements. Examples:
> =A0#
> =A0# =A0payments.sum { |p| p.price * p.tax_rate }
> =A0# =A0payments.sum(&:price)
> =A0#
> =A0# The latter is a shortcut for:
> =A0#
> =A0# =A0payments.inject { |sum, p| sum + p.price }
> =A0#
> =A0# It can also calculate the sum without the use of a block.
> =A0#
> =A0# =A0[5, 15, 10].sum # =3D> 30
> =A0# =A0["foo", "bar"].sum # =3D> "foobar"
> =A0# =A0[[1, 2], [3, 1, 5]].sum =3D> [1, 2, 3, 1, 5]
> =A0#
> =A0# The default sum of an empty list is zero. You can override this defa=
ult:
> =A0#
> =A0# =A0[].sum(Payment.new(0)) { |i| i.amount } # =3D> Payment.new(0)
> =A0#
> =A0def sum(identity =3D 0, &block)
> =A0 =A0if block_given?
> =A0 =A0 =A0map(&block).sum(identity)
> =A0 =A0else
> =A0 =A0 =A0inject { |sum, element| sum + element } || identity
> =A0 =A0end
> =A0end

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. =A0It 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. =A0Yes 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. =A0It
> 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. =A0Having said that perhaps a better name for the method
> might be
>
> =A0 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

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