solve: 1011.Capacity To Ship Packages Within D Days#44
Conversation
naoto-iwase
left a comment
naoto-iwase
left a comment
There was a problem hiding this comment.
読みやすかったです。
実用を見据えた書き方に感じて素晴らしいと思いました。
| Rustのbinary_search_byはスライスに対する操作（配列を実体化する必要がある）なので、空間計算量O(n)となるので注意が必要。 | ||
| Rustでは標準ライブラリでRangeに対する二分探索APIがない模様。 | ||
| bisectionという外部クレートを見つけたが、Rangeに対する二分探索は行えないようだった。 | ||
| IssuesやPull Requestを見ていたところ、二分探索アルゴリズムにおいて中間を計算するロジックがオーバーフローする可能性があるというIssueがあった。 | ||
| https://github.com/SteadBytes/bisection/issues/2 | ||
| コーディング練習会でもレビューで良く指摘があるのであるあるなんだなと思った。 | ||
| Issueの中でこの二分探索アルゴリズムに関するGoogle Researchの記事へのリンクが貼られており、過去にJDK用に実装された二分探索アルゴリズムでも同様のバグがあったのことで面白いと思った。 | ||
| https://research.google/blog/extra-extra-read-all-about-it-nearly-all-binary-searches-and-mergesorts-are-broken/ | ||
| ちなみに、bisectionクレートに依存しているライブラリにastral-sh/uv(Pythonパッケージ、プロジェクト管理ツール)があることに気付いた。 | ||
| uvは昨今のPythonパッケージ管理ツールとしてデファクトスタンダードになっている感じもあるので、依存しているクレートにバグが有ることを報告して実装を差し替えるなりのPull Requestチャンスかなと思って調べていたら、すでにPull Requestが行われていた。 | ||
| https://github.com/prefix-dev/async_http_range_reader/pull/18 |
There was a problem hiding this comment.
このあたり非常に面白く読ませていただきました！
Rustでは標準ライブラリでRangeに対する二分探索APIがない模様。
bisectionという外部クレートを見つけたが、Rangeに対する二分探索は行えないようだった。
依存しているクレートにバグが有ることを報告して実装を差し替えるなりのPull Requestチャンスかなと思って調べていたら
使っている道具の仕様（言語仕様やライブラリの特性）を深掘りしたくなる気持ちや、ライブラリ実装のバグの可能性を察知して直そうとする姿勢、いずれもSWEのマインドセットそのもので、とてもいいなと思いました。
There was a problem hiding this comment.
Range は範囲を意味していて、iterate することは想定されているもののランダムアクセスができる想定はないのでしょう。Rust の slice は view なので作ってもコピーされません。こちらが想定される二分探索の対象でしょう。
| return 0; | ||
| } | ||
| if weights.iter().any(|weight| *weight < 0) { | ||
| panic!("weights must contain only positive values"); |
There was a problem hiding this comment.
条件とメッセージを整合させたいと思いました。
条件の方を <= 0 にするかメッセージを “non-negative” にすると良いと思います。
| max_capacity_of_day.try_into().expect(&format!( | ||
| "max_capacity_of_day downcast failed. max_capacity_of_day: {}", | ||
| max_capacity_of_day | ||
| )) |
There was a problem hiding this comment.
趣味の範囲ですが、Rustのexpectは、エラーが発生していなくても引数が評価されるそうです。したがって、この書き方だと成功時でも毎回format!マクロが実行され、文字列のアロケーション（メモリ確保）が発生してしまいます。
代案としてはunwrap_or_else(|_| panic!(...))が使えそうです。
| - lower_capacity,upper_capacityの初期化をbefore->afterに変更した。 | ||
| - 問題の文脈からして船の最小積載重量ロジックは頻繁に呼び出されると判断。 | ||
| - 頻繁に仕様変更が発生して読み書きされる部分では無いと判断。 | ||
| - 以上の理由から人によって頻繁に読み書きされず、頻繁に呼び出されそうなメソッドなので可読性を少し犠牲にして一度の走査で計算する方向にした。 |
There was a problem hiding this comment.
趣味の範囲ですが、個人的には確かに1回の走査で済む分、局所的な計算量的にはO(n)の定数倍の改善ですが、全体O(n log m)への影響はほぼないとみなせるので、可読性を重視してbeforeのスタイルで書きたいと思いました。独立してできる2つのことをくっつけて書いているのが読みにくさそのものでもありますね。
ただ、この辺りのトレードオフをわかっているように見えたので大丈夫だろうと思いました。
3 participants
問題: 1011. Capacity To Ship Packages Within D Days
次に解く問題: 50. Pow(x, n)
ファイルの構成:
./src/bin/<各ステップ>.rs