Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor implementation of array-like literals #5703

Open
straight-shoota opened this issue Feb 10, 2018 · 8 comments
Open

Refactor implementation of array-like literals #5703

straight-shoota opened this issue Feb 10, 2018 · 8 comments

Comments

@straight-shoota
Copy link
Member

An array-like literal such as Foo{1, 2, 3} is currently rewritten to create a new instance and sequentially invoke << for each item. So something like this:

tmp = Foo.new
tmp << 1
tmp << 2
tmp << 3
tmp

This implementation works well for regular collection types like Set or custom array aliases.

When implementing #5635 (adding a Path type) it showed some drawbacks. Literals like Path{"path", "to", "file"} are much slower than creating an instance like Path.new("path", "to", "file"). The latter invokes join with all arguments at once which is much faster than invoking join once for each component.
Another issue with this is that you can't have really immutable objects.

@asterite suggested to simply rewrite T{x, y, z} to T.new({x, y, z}) or T.literal(x, y, z). This could even be optional: If a class implements a literal method, it is used. Otherwise, it could keep the current behaviour with <<.

It would be nice to find a similar solution for hash-like literals, although this is a bit trickier. A dynamic-sized list of items can easily be passed to a method as a tuple but hash-like literals contain key-value pairs. But I guess it could work if each kv-pair is converted to a tuple, so that T{"a" => 1, "b" => 2} is rewritten to T.literal({{"a", 1}, {"b", 2}}).

@Sija
Copy link
Contributor

Sija commented Feb 11, 2018

Why not use Path.[] for that?
Writing Path["path", "to", "file"] seems at least as concise and readable.

@RX14
Copy link
Contributor

RX14 commented Feb 12, 2018

No, why not just use Path.new("path", "to", "file"). Or I'd even prefer that constructor named Path.join. Path isn't an array and it shouldn't be treated like one.

@straight-shoota
Copy link
Member Author

A path consists of a sequence of components... I don't think this wouldn't qualify as a suitable use case for an array-like literal.

Of course you could just use Path.new (or join). Path{} is just a bit shorter. And I like that.

If this is not wanted for Path, you could go one step further: why are there array and array-like literals at all? Array literals could also be written as Array.new(1, 2, 3) (assuming it hat such a constructor accepting a splat tuple).

@asterite
Copy link
Member

There's an array constructor with two integers: size and default value. So that won't work.

@straight-shoota
Copy link
Member Author

Yeah that's not entirely thought out, I'm just trying to make a point against that. It could probably just use a different name. Or a symbol such as [] like @oprypin's suggested regarding Path (this is currently implemented in the Path PR). This could also be used for Array, Set etc.

@RX14
Copy link
Contributor

RX14 commented Feb 13, 2018

Arrays, tuples, named tuples, and to a lesser extent hashes are by far the most common data types and are worth special syntax. I'm not sure less-generic or less-common types should have the same. I'm actually fine with this idea of changing the way generic array literals work, its just I would argue against it being applied to path.

@al6x
Copy link

al6x commented Dec 27, 2019

Hit that issue when tried to create immutable (so it can't have << method) TimeSeries collection. I found a hack to use [] instead of {} (steal it from Immutable shard)

class TimeSeries(T)
  def initialize(@array = Array(T?).new); end

  def self.[](*values : T | T?)
    Serie(T).new values.to_a.map{|v| v.as(T?)}
  end
end

TimeSeries[1, 2]

Yet, it would be nice to have an uniform way to declare collections always the same way Set{1, 2}, TimeSeries{1, 2, 3} and not remember that sometimes you need to use { and sometimes [.

@Sija
Copy link
Contributor

Sija commented Dec 27, 2019

Yet, it would be nice to have an uniform way to declare collections always the same way Set{1, 2}, TimeSeries{1, 2, 3} and not remember that sometimes you need to use { and sometimes [.

That can be achieved at the API level, not the language semantics though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants