2007/11/13, furtive.clown / gmail.com <furtive.clown / gmail.com>:
>
> I forgot to include this option:
>
>    t = stems.map { |f|
>       f + "." + guess_extension(f)
>    }
>    all_files3 = transform2(source_files + transform1(t)).map { |f|
>       File.join(dir, f)
>    }
>
> Introducing needless temporaries on the same scope level as the target
> variable makes understanding and maintenance harder.  Look again:

IMHO introducing temporary variables with proper names can go a long
way to make this piece much more readable. (The same holds true for
method names.)

>    all_files = stems.map { |f|
>       f + "." + guess_extension(f)
>    }.as { |t|
>       transform2(source_files + transform1(t))
>    }.map { |f|
>       File.join(dir, f)
>    }
>
> This clearly communicates that the purpose of the block chain is to
> obtain a value for all_files.  From beginning to end, we understand
> the intent at a glance.

Frankly, I don't.

>  In the former case, the intent is muddled by
> a useless variable floating around.  Multiply this example by 10 and
> it quickly becomes the difference between beauty and travesty.

Obviously preferences differ.  Although I can only guess what all
those methods do, I would prefer this variant:

all_inputs = source_files +
  transform1( stems.map { |f| f + "." + guess_extension(f) } )
all_files = transform2( all_inputs ).map {|f| File.join dir, f}

Note, if transform2 and transform1 do not need the whole collection I
would change their implementation to transform a single value only
which then would make the whole story a lot simpler.  The current
approach seems quite complex to me but without knowing what all this
is supposed to do I have no better variant to offer.

Kind regards

robert

-- 
use.inject do |as, often| as.you_can - without end