Make Hash (and others) yield tuples in each #2781
Conversation
That might make sense for the version that takes an argument, but it falls apart for the argument less version. The perspective to take here is not that the argument gives the default value for |
|
But |
|
why you not consider {}, instead of (), i think it more correct: hash = {1 => 2}
hash.each do |{key, value}|
key # => 1
value # => 2
end |
|
@kostya you can do: ary = [[1, 2], [3, 4]]
ary.each do |(x, y)|
x # => 1
y # => 2
endBasically, you can unpack anything that responds to Because @jhass I can probably remove the |
|
I think the current version visualizes like this While yours visualizes more like That is people will a lot less likely think (and explain it) as "the argument is simply prepended to the collection", but rather "With an argument, it becomes the first value for memo, without an argument the first value becomes the first value for memo and the other argument is actually the second value". I think that explanation is more complicated especially in hindsight to what reduce does, passing its return value back as memo. Another way to argue this is visualizing the reduction: I constantly have to flip the arguments in my mind compared to them just reducing down left to right. |
|
Perhaps we should rename |
asterite
force-pushed
the
feature/hash_tuple
branch
from
252aa00 to
7064fe8
Compare
|
Closed in favor of #2783 |
This is an alternative to #2778, proposed in this comment.
Basically, now you have to do:
If you do
do |key, value|,keywill be a tuple andvaluewill benil. We can of course discuss if this should actually be a compile error instead of binding the variable tonil.I also changed the order in
Enumerable#reduceto beelem, memo, which is more consistent with everything else.HTTP::HeadersandENVare also changed to this new behaviour, and now includeEnumerable.Now, my opinion about this vs. #2778 . If you look at the diff you'll see most changes change
|key, value|with|(key, value)|. To me, this adds no information at all, and feels redundant and is definitely more tedious to type. It's hard to get confused if auto-unpacking would be turned on, becausekeyandvalueare self-describing names, so someone reading the code would know what to expect.The same thought applies to changes from
|key, value, index|to|(key, value), index|, it feels like the language is forcing me to add parentheses for no real reason (no more type safe, and I could still use the explicit unpacking if I wanted to).Additionally, in some places there's
each_with_index do |elem, index|and one might wonder why it's not|(elem, index)|instead. Maybe it's because the iterated element, and the index, are separate entities. Should we changeArray#zipto return tuples, or yield two elements? If a class isn't Enumerable but yields two things in a method, should one use a tuple or not? All these questions start to appear. Auto-unpacking make them all go away, and leaves the door open for the programmer, on the call site, when writing the block, to choose explicit unpacking, or implicit auto-unpacking.This PR of course breaks most code out there, as they need to upgrade to the new
eachmethod. Auto-unpacking, on the other hand, is almost backwards compatible, unless you did something likehash.each { |key| ... }(didn't happen in the bunch of projects I tested).If Crystal is to be a pragmatic, less noisy, practical language, I think #2778 is the way to go. But of course this approach isn't that bad either, it just feels a bit more cumbersome to code like this, though it's always explicit and there's no "magic".