impl Iterator → impl IntoIterator
#11
Conversation
|
なるほどです！ 言われてみれば、なんとなくこちらの型指定の方が多いかもですね。 後学のためにお聞きしたいのですが、この型指定にすることで得られるメリットが明らかにある場合教えてくださると嬉しいです。また、このような「こう書いた方がより良い」という指針はどこから得られましたか？ なんとなく、確かに標準ライブラリはこんな感じの引数にしてあるなと思い当たったのですが……。 |
利点としては #12 もですが、 #9 のレビュー中だったらsuggestion投げていたかなというくらいの温度感でいます。
あえて出典を挙げるならRust API GuidelinesのC-GENERICでしょうか? まあこれはpublicなAPIについての話ですが。 |
|
なるほどです、ありがとうございます。
これは確かに私もレビューしているときに同様のことを感じました。しかし「変えなければいけない」というほどの理由が自分の中にはなかったのでそのままにした感じです。役割を十分果たしていればそれで問題ない、と判断しました。この関数は他の場所で再利用される場面はほぼないと考えて良さそうだったのと、仮にあったとしてもそのときに適切な形に変えれば良い、という判断もありました。 （正直にいうと「コミュニケーションの数を最適化したい」という気持ちもありました。個人的に最近忙しくなってきているので、時間的・体力的なリソースを潤沢に振り向けられなくなってきています。ここはちょっと申し訳なさがあります。明らかに重大な議題であったならそれでも十分議論を尽くすつもりでいますが……）
という意見は確かにそうだな、と感じ勉強になりました。参考リンクもありがとうございます。ただ上に書いた観点からは、個人的には「変更前でも変更後でもどちらでも良い」という印象でした。 （誤解を避けるため念のためお伝えしますが、今回のように「この方がより良いかも・より自然かも」という提案や視点をいただけること自体は非常にありがたいと思いました。） |
なるほど。レビュー判断としてそのままにしたんですね。
そこはお時間取ってしまって申し分けなかったなーと思ってます。 実はこのPRは #12 と合体して"Fix up #9"のようなタイトルで出そうと思ってたのですが、qwertyさんが書いていた部分で一箇所だけ 変えるべき理由は特に無い...けど変えない理由も特に無いし、レビュー中ならsuggestion出してた。 #9 マージ直後の今ならまだセーフなのでは?という思考でこのPRを出しました。こう、なんというか3秒ルール的な感じで (本物の3秒ルールは衛生的に駄目ですが)。 |
|
あー、なるほどです。本来は先の PR と同時に改善しようとしていた、という事情がわかりました。なんにせよ、提案自体はなるほどなと思ったのでありがとうございます！ |
タイミングが今で申し訳無いのですが、 #9 で入ったコードに対する提案です。
こちらの方がidiomaticかと思います。