Please open an issue to talk about this, what the community thinks about at, fetch and [].
When we will found a consensus solution on this topic then the changes could be made :)

Please open an issue to talk about this, what the community thinks about at, fetch and [].
When we will found a consensus solution on this topic then the changes could be made :)

Why do you need for an issue for this if there is already PR?

It's even available by /issues link instead of /pull: https://github.com/crystal-lang/crystal/issues/6296 (GitHub automatically convert regular link to <a ...>#6296</a>)

(UPD: /issue fixed with /issues)

And PRs in search results along with issues: https://github.com/crystal-lang/crystal/search?q=indexable+at&type=Issues

Anybody can discuss anything here about code, approach, needing.

By the way, I like this PR. It makes the API a bit more consistent with Hash, which also has fetch, despite the fact that it's a breaking change.

Huum, fetch for arrays disturbs me, because fetch is associated to hashmaps in my head. I've been wondering, why do we have at or fetch without a block or default value? Do they have any value compared to []? I even wonder whether unsafe_at(i) is really useful. Isn't it just aliasing to_unsafe[i] usually?

I'd be fine with .fetch(index) { default } only.

Well, an array could be viewed as a mapping from integers to some values, with missing keys being those outside the array's range. For example in PHP arrays are both arrays and hashes (not that it means something, but it's a nice mathematical view).

Can at and fetch be aliases?

They can, but we try to avoid aliases in Crystal.

Is there a reason to this syntax be invalid?

array[100] { nil }

It looks a bit weird. And it's not valid Ruby syntax either (maybe for a reason).

I like the idea. Both Indexable and Hash describe collections that allow directly accessing it's members by a key. It makes sense to use the same names for this concept.

For reference: in Java both Map and List have a get method. I'd like #get for Array and Indexable in Crystal, too. But I don't think it would necessarily be better than #fetch and certainly not worth breaking both APIs.

#at always sounded a bit strange to me and I find it hard to make sense of it.

We could think about renaming values_at as well. It doesn't necessarily need to change because the name makes sense even with out #at. But maybe #fetch_all would be a more intuitive idea.

We could think about renaming values_at as well

For me, at is for specific position (like []), and fetch is more softer, like "get an element or return/do something (default)".

It doesn't necessarily need to change because the name makes sense even with out #at.

Are you about renaming #values_at to #values? I disagree:

{ a: 1, b: 2, c: 3 }.values_at(:c, :a) #=> [3, 1]
{ a: 1, b: 2, c: 3 }.values #=> [1, 2, 3]

And #values_at sounds more correct than #values(:c, :a).

Note that we could add a[1] { ... } as a valid syntax, though. It would at least solve the issue of finding the perfect name for the operation :-P

@AlexWayfer No, I meant renaming #values_at to something else (for example #fetch_all), not just dropping the _at suffix.

@AlexWayfer No, I meant renaming #values_at to something else (for example #fetch_all), not just dropping the _at suffix.

Oh, OK, I understood, thanks.

It may be fetch + at = #fetch_at. 😅

This is incorrect, I think. #fetch_from may be more correct.

Just for notice: checks fail not because of code.

This is valid Ruby (2.5) code:

def [](idx, &block)
  puts idx + block.call
end

self[1] { 2 }

https://repl.it/repls/GreedyConsciousJava

@hugoabonizio Cool! Let's add that to Crystal too.

Here's an initial diff, if someone wants to take care of this:

diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr
index ecdf0db3c..4e52e658d 100644
--- a/spec/compiler/parser/parser_spec.cr
+++ b/spec/compiler/parser/parser_spec.cr
@@ -124,6 +124,10 @@ module Crystal
     it_parses "foo[] /2", Call.new(Call.new("foo".call, "[]"), "/", 2.int32)
     it_parses "foo[1] /2", Call.new(Call.new("foo".call, "[]", 1.int32), "/", 2.int32)
     it_parses "[1] /2", Call.new(([1.int32] of ASTNode).array, "/", 2.int32)
+    it_parses "foo[1] { 2 }", Call.new("foo".call, "[]", args: [1.int32] of ASTNode, block: Block.new([] of Var, 2.int32))
+    it_parses "foo[1]? { 2 }", Call.new("foo".call, "[]?", args: [1.int32] of ASTNode, block: Block.new([] of Var, 2.int32))
+    assert_syntax_error "foo[1, &x] { 2 }", "block already specified with &"
+    assert_syntax_error "foo[1, &x]? { 2 }", "block already specified with &"
 
     it_parses "!1", Not.new(1.int32)
     it_parses "- 1", Call.new(1.int32, "-")
diff --git a/spec/std/indexable_spec.cr b/spec/std/indexable_spec.cr
index 344e5bda9..e2110bebb 100644
--- a/spec/std/indexable_spec.cr
+++ b/spec/std/indexable_spec.cr
@@ -114,4 +114,12 @@ describe Indexable do
     return_value.should eq(indexable)
     last_element.should eq(3)
   end
+
+  {% if compare_version(Crystal::VERSION, "2.5.1") > 0 %}
+    it "does [] with block" do
+      indexable = SafeIndexable.new(5)
+      (indexable[1] { 2 }).should eq(1)
+      (indexable[10] { 2 }).should eq(2)
+    end
+  {% end %}
 end
diff --git a/src/compiler/crystal/syntax/parser.cr b/src/compiler/crystal/syntax/parser.cr
index 6d2d509f2..6bfef1b81 100644
--- a/src/compiler/crystal/syntax/parser.cr
+++ b/src/compiler/crystal/syntax/parser.cr
@@ -715,13 +715,6 @@ module Crystal
           @wants_regex = false
           next_token
 
-          if call_args
-            args = call_args.args
-            block = call_args.block
-            block_arg = call_args.block_arg
-            named_args = call_args.named_args
-          end
-
           if @token.type == :"?"
             method_name = "[]?"
             next_token_skip_space
@@ -730,6 +723,16 @@ module Crystal
             skip_space
           end
 
+          if call_args
+            args = call_args.args
+            block = call_args.block
+            block_arg = call_args.block_arg
+            named_args = call_args.named_args
+          end
+
+          curly_block = parse_curly_block(block_arg)
+          block = curly_block if curly_block.is_a?(Block)
+
           atomic = Call.new(atomic, method_name, args: (args || [] of ASTNode), block: block, block_arg: block_arg, named_args: named_args, name_column_number: column_number).at(location)
           atomic.name_size = 0 if atomic.is_a?(Call)
           atomic
diff --git a/src/indexable.cr b/src/indexable.cr
index d5dc7df4a..84149f609 100644
--- a/src/indexable.cr
+++ b/src/indexable.cr
@@ -73,6 +73,18 @@ module Indexable(T)
     at(index)
   end
 
+  # Returns the element at the given *index*, if in bounds,
+  # otherwise executes the given block and returns its value.
+  #
+  # ```
+  # a = [:foo, :bar]
+  # a[0] { :baz } # => :foo
+  # a[2] { :baz } # => :baz
+  # ```
+  def [](index : Int)
+    at(index) { yield }
+  end
+
   # Returns the element at the given *index*.
   #
   # Negative indices can be used to start counting from the end of the array.

Adding the [] methods with a block variant is currently supported, it's just the [] { ... } syntax that's missing. So we can add all of the functionality for the next release. I just don't have time to implement all of the block variants (Hash, String, and many others would need them). Eventually we could remove the at and fetch variants, though I'm not sure because we still need the variant that accept a second argument as the default value (instead of a block with the default value).

/cc #6324

@asterite a[x]? || "default value" is short enough, honestly. a[x] { "default value" } if your value type contains nil.

@RX14 I guess that's fine. We would eliminate at and fetch, and just keep [] and []?, which is much simpler (although very different than Ruby with all its aliases).

If someone want to go for it, you can use my diff above as a start :-)

@asterite a[x]? || "default value" is short enough

Doesn't work for falsey values, I guess.

We would eliminate at and fetch, and just keep [] and []?, which is much simpler (although very different than Ruby with all its aliases).

With blocks?

@hugoabonizio Cool! Let's add that to Crystal too.

Let's add the ability to define #[](arg, &block) method?

Array#[] in Ruby doesn't receive blocks:

Doesn't work for falsey values, I guess.

That's where you would use the block variant.

Array#[] in Ruby doesn't receive blocks

Yes, in Ruby 2.5.1, although it seems they forgot to document it.

Yes, in Ruby 2.5.1, although it seems they forgot to document it.

No, they didn't. It's valid for some reason, but doesn't work as expected.

https://repl.it/repls/SmartQuestionableLegacy

Having the triplet [], [] {} and []? and removing fetch and at methods sounds good to me!

I don't like this idea at first sight.

foo.fetch(key, default) is a simple and easy to understand interface. Replacing that with foo[key]? || default or foo[key] { default } seems like a step backwards. The first one accuratly describes it's purpose, but whether it can be used depends on the return type. This is bad. If for some reason the return type changes to include falsey values, you'd have to first notice this doesn't work anymore and change it everywhere.
Well, we could maybe just use foo[key] { default } everywhere.

But IMHO foo.fetch(key, default) (or even more expressive: foo.fetch(key, default: default) is way better. Please consider these consequences before removing it.

#[key, default] is also an option.
At least when using signs there is no wording debate 😄

Is it OK with change unsafe_at to unsafe_fetch?

@AlexWayfer yes +1

Source

Let's remove fetch with just one argument.

Source.

No we don't have fetch with one argument, and we have unsafe_fetch with just one argument. Is it OK?

Conflicts resolved.

Conflicts resolved again.

Sorry, fails fixed.

Is it possible to add this PR to 0.27.0?

@bcardiff I think this would be nice in 0.27.0 too