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

feat: 関数の省略された引数がexistsでfalseを返すように #636

Closed
wants to merge 9 commits into from

Conversation

FineArchs
Copy link
Member

@FineArchs FineArchs commented Apr 25, 2024

What

関数宣言時に指定した引数が実行時に与えられなかった時、これまでは中身が定義外の値が代入されていましたが、それをそもそも変数として追加されないように変更します。
破壊性が低いと考えられるためmasterに入れます。

Why

#635
なお #475 がマージされた場合、省略可能でない引数が省略されるとエラーになるようになるためこのPRは無効化されます。
nextが来るまでの誤魔化しとしての利用を想定しています。

Additional info (optional)

Copy link
Contributor

@marihachi marihachi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rejectしたいです

@FineArchs
Copy link
Member Author

FineArchs commented Apr 26, 2024

Rejectしたいです

では反対1名として扱いますが、理由を伺ってもいいですか?

@salano-ym
Copy link
Member

これを前提に書いたコードはnextになった時に破壊的になる?

@FineArchs
Copy link
Member Author

FineArchs commented Apr 26, 2024

はい、まあそれはそうですね

@salano-ym
Copy link
Member

salano-ym commented Apr 26, 2024

今から壊れる要素を増やすのはあまり良くないと思うのでやはり個数チェックで弾く方が安全かもしれません

@FineArchs
Copy link
Member Author

個数チェックの追加となると破壊的になりそうな気がしますが今からmasterに追加します?

@salano-ym
Copy link
Member

nextまでの誤魔化しなら今はNULLを入れておいてnextでは?指定必須という形にするのもあり?

@FineArchs
Copy link
Member Author

nextまでの誤魔化しなら今はNULLを入れておいてnextでは?指定必須という形にするのもあり?

これもnextになった時に破壊的要素になるはなりますが、基本的に?を入れるだけで修正できるという点でexistsでどうこうよりはベターですね
引数忘れでもエラーが出ないことで変に動くプログラムが生まれるみたいな懸念はありますが、一時的な措置ですしCHANGELOGに警告だけ書いておきましょうか

@salano-ym
Copy link
Member

salano-ym commented Apr 26, 2024

「ユーザー定義関数の引数省略は正式実装ではないので非推奨」という旨の警告が必要ですね

@marihachi
Copy link
Contributor

existsでfalseを返すという挙動が直感的ではないというのが反対の理由です。
引数の変数自体は存在しているため、別のアプローチの方が良いと思います。

@FineArchs
Copy link
Member Author

逆に私がそのアプローチにしようとした理由は、「existsでfalseを返す値」をJavaScriptでいうundefinedの代わりにできたら利便性と安全性を両立できるかもしれない、という構想があったからです。
ただまあ「あるのにない」というわかりづらさがあるのは確かでした

@FineArchs FineArchs closed this May 14, 2024
@FineArchs FineArchs deleted the fn-args branch May 14, 2024 11:15
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

Successfully merging this pull request may close these issues.

3 participants