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

Rename NamedTupleLiteral#[] to NamedTupleLiteral#[]? #5766

Closed
bew opened this issue Mar 4, 2018 · 5 comments
Closed

Rename NamedTupleLiteral#[] to NamedTupleLiteral#[]? #5766

bew opened this issue Mar 4, 2018 · 5 comments

Comments

@bew
Copy link
Contributor

bew commented Mar 4, 2018

I have a macro-land array of NamedTupleLiteral, each representing a field of data to parse for a binary protocol (this is my usecase).

Here is an example:

MACRO_VAR__X_fields = [] of _

macro fixture_fields
  {% MACRO_VAR__X_fields << {type: :pad_bytes, by_n: 5} %}
  {% MACRO_VAR__X_fields << {type: :card16, name: "data_len", save: false} %}
  {% MACRO_VAR__X_fields << {type: :string, name: "important_data", save: true} %}
end

fixture_fields

Now I need to get all the fields that needs to be saved:

{% elems_to_save = MACRO_VAR__X_fields.select { |e| e.keys.includes?(:save.id) && e[:save] } %}

This works (https://carc.in/#/r/3o3w).

But with NamedTupleLiteral#[]?, this could be rewritten as:

{% elems_to_save = MACRO_VAR__X_fields.select { |e| e[:save]? } %}
# or even
{% elems_to_save = MACRO_VAR__X_fields.select &.[:save]? %}

Which feels way better, and simpler to understand.

It is currently missing, and I think it should be added! 😃

I can send a PR if you agree

@bew
Copy link
Contributor Author

bew commented Mar 4, 2018

Actually, I just realized that NamedTupleLiteral#[] is NamedTupleLiteral#[]?, so:

{% nt = {a: 1}; puts nt[:non_existant] %} # => nil

https://carc.in/#/r/3o4b

I think the documentation should be changed, or the method renamed to what it really is [] => []?.

@bew bew changed the title Add NamedTupleLiteral#[]? Rename NamedTupleLiteral#[] to NamedTupleLiteral#[]? Mar 29, 2018
@straight-shoota
Copy link
Member

Documentation should be fixed, but it makes no sense to rename to #[]? without having #[].

@jhass
Copy link
Member

jhass commented Apr 3, 2018

Do we have foo? anywhere in macro land? I thought we didn't because it's much more script like and runtime type checking anyway.

@ysbaddaden
Copy link
Contributor

@jhass is right: methods are nilable by default in macros, unlike runtime methods that raise by default and may have a nilable alternative.

Close?

@RX14
Copy link
Contributor

RX14 commented Apr 3, 2018

Close.

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

No branches or pull requests

5 participants