-
Notifications
You must be signed in to change notification settings - Fork 0
1011. Capacity To Ship Packages Within D Days #51
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
base: main
Are you sure you want to change the base?
Conversation
| CXX = g++ | ||
| # コンパイルオプション | ||
| CXXFLAGS = -std=c++17 -Wall -Wextra -O2 | ||
| CXXFLAGS = -std=c++17 -Wall -Wextra -O2 -I/Library/Developer/CommandLineTools/SDKs/MacOSX15.2.sdk/usr/include/c++/v1/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CXXFLAGS、二つありますね
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@usatie 細かくレビュー頂きありがとうございます。Binary Searchに対しては特に苦手意識を持っているため助かります。
|
|
||
| int Solution::shipWithinDays(vector<int>& weights, int days) { | ||
| int min_capacity = *max_element(weights.begin(), weights.end()); | ||
| int max_capacity = accumulate(weights.begin(), weights.end(), 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::を省略しないでほしいと個人的には感じます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
命名ですが、あくまで二分探索を行う上での探索範囲の中でのmin/maxだと思うので、それを明示しないとわかりづらいと感じました。関数名からはこの関数が二分探索を行うことは自明ではないので、いきなり出てくると認知負荷があります。代入の右辺の式を読んだら変数の意味がわかるのかと思って読んでもこの二行単体だとよくわかりませんし。後まで読み進めて、二分探索であることがわかってようやく意味がわかって、もう一度この二行を読むことになると思います。
二分探索である旨のコメントを書くか、left/rightやlo/hiなどと命名されていればああ二分探索なのね、とすぐわかると思いますが。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@usatie
LeetCodeではstdが不要だったので気にせず使ってました🙇♂️
調べてみたところ以下のような意見がございました。
using namespace std;は小さな規模のプログラムを書く際には便利ですが、 規模が大きくなると名前の衝突を避けることが大変になるため、 グローバルなスコープにusing namespace std;を書くのは避けるべきです。
https://atcoder.jp/contests/apg4b/tasks/APG4b_ag?lang=ja#:~:text=using%20namespace%20std%3B%20%E3%81%AF%E5%B0%8F%E3%81%95%E3%81%AA,%E3%81%AE%E3%81%AF%E9%81%BF%E3%81%91%E3%82%8B%E3%81%B9%E3%81%8D%E3%81%A7%E3%81%99%E3%80%82
可読性にも関わるのですね。
https://yumetodo.hateblo.jp/entry/2017/12/15/221648
宣言と実体を分ける練習ではstdは省略しないようにします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
競技プログラミングは、東大にあったサークルの名前に由来します。
その影響力があまりにも強かったために、Competitive Programming のことを「競技プログラミング」と呼ぶようになりました。(英国ラグビー町にあるラグビー校でルールが決まったのでラグビーと呼ぶようになったようなものです。)
競技プログラミング同好会前後の世代は変なプラクティスを色々開発した上に、困ったことにそのあたりの世代は、ほぼ Google と IBM に入っていくものだから、下の世代は虎の威を借る狐をしているというわけです。
あのころ Google と IBM に多くいたことは、問題文でネタになるくらいでしたからねえ。
https://www.utpc.jp/2011/
というわけで、私達の世代の作った変なプラクティスが多いので、信用しないほうがいいです。
| leftをmax_weight、rightを配列の最後の要素sum_weightとする。 | ||
|
|
||
| 4.ループ不変条件の設定 | ||
| left < right、[left, right)に解が含まれている。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ryotaroさんのコードでは、ループ終了時にleft = rightになるので、ループ不変条件という言葉遣いは誤解を招くのではないでしょうか。
このコードの場合だったら、「開区間 閉区間 [left, right] にdays以内に運ぶことが出来る最小のキャパシティが含まれる」がループ不変条件ではないでしょうか。
ループが終了する際には、ループ不変条件とループ終了条件の両方が真であることが保証される。
https://ja.wikipedia.org/wiki/%E3%83%AB%E3%83%BC%E3%83%97%E4%B8%8D%E5%A4%89%E6%9D%A1%E4%BB%B6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
「閉区間」でした。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 存在しないためleftをmiddle + 1に更新する。 | ||
|
|
||
| capacity_of_the_ship[middle]がdays以内に運ぶことが出来る場合、 | ||
| middleを区間内に含めるため、right = mid とし区間を狭める。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
実際middleをrightとして区間内に含めるならば、右は開区間ではないですよね。
| #include <vector> | ||
| #include <numeric> | ||
| #include <iostream> | ||
| using namespace std; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.h に using name std; を書くと、これを include したすべての .cc が影響を受けます。
C++ は、型かどうかでパースが変わるので、影響範囲がとても大きいです。
.cc に書くのは、まだいいですが。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oda
例えばstd::countと変数名のcountがぶつかる場合などがあるのですね🙇♂️
| @@ -0,0 +1,40 @@ | |||
| class Solution { | |||
| public: | |||
| int shipWithinDays(vector<int>& weights, int days) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const vector& と const つけてもいいかもしれません。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oda
レビューありがとうございます。step1を終えてつけていないことに気づきました🙇♂️
|
|
||
| #include <vector> | ||
| #include <numeric> | ||
| #include <iostream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include はアルファベット順にソートすることが多いですね。
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
Within each section the includes should be ordered alphabetically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あ、「include はアルファベット順にソートする」というのは、種類別に分けた後の話です。
あと、この iostream は .cc では使っていても .h では使っていませんね。
こういう場合は、.cc で include します。
C++ の include は「ソースコードをそこに貼り付ける」という意味なのでコンパイルが重くなります。
(Java, Python などの import は貼り付けません。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oda
ドキュメントありがとうございます。確認しました。
C++ の include は「ソースコードをそこに貼り付ける」という意味なのでコンパイルが重くなります。
必要な箇所でincludeするようにしました。
| int total_weight = 0; | ||
| for (auto weight : weights) { | ||
| total_weight += weight; | ||
| if (total_weight > capacity) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好みの問題になります。
total_wegiht > capacityを超えた場合に再度total_weight = weightとしているの直感的ではない気もしてて、
選択肢として、
先にtotal_weight + weight > capacityを確認し、加えるのが可能であればtotal_weight += weightして次のweight, そうでなければtotal_weight = weight, days_required +=1 と言う書き方でも良いかと思いました。(Pythonだとcontinueするイメージです。)
| } | ||
|
|
||
| private: | ||
| bool IsLoadable(vector<int>& weights, int capacity, int required_days) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weightsの中にcapacityを超えるweightが一つでもあればfalseになるべきですが、このコードだとtrueになりうりますね
実際は、leftの初期値をmax(weights)にしているので耐えていますが、現在この初期値がプラスマイナス1ずれるとバグるので、すこし不安な気持ちになりました
問題へのリンク
https://leetcode.com/problems/capacity-to-ship-packages-within-d-days/description/
問題文(プレミアムの場合)
備考
次に解く問題の予告
3. Longest Substring Without Repeating Characters
フォルダ構成
LeetCodeの問題ごとにフォルダを作成します。
フォルダ内は、step1.cpp、step2.cpp、step3.cppとmainディレクトリとmemo.mdとなります。
mainディレクトリ 内で宣言と実体を分ける練習をしました。
memo.md内に各ステップで感じたことを追記します。