Skip to content

Conversation

@Ryotaro25
Copy link
Owner

問題へのリンク
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内に各ステップで感じたことを追記します。

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/
Copy link

Choose a reason for hiding this comment

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

CXXFLAGS、二つありますね

Copy link
Owner Author

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);
Copy link

Choose a reason for hiding this comment

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

std::を省略しないでほしいと個人的には感じます。

Copy link

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などと命名されていればああ二分探索なのね、とすぐわかると思いますが。

Copy link
Owner Author

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は省略しないようにします。

Copy link

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)に解が含まれている。
Copy link

@usatie usatie Feb 25, 2025

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

Copy link

Choose a reason for hiding this comment

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

「閉区間」でした。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@usatie
ループ不変条件とleftとrightの定義がとても曖昧でした。usatieさんの指摘の通り確かに閉区間でした。
文章を修正してみました。
efaa4e1

存在しないためleftをmiddle + 1に更新する。

capacity_of_the_ship[middle]がdays以内に運ぶことが出来る場合、
middleを区間内に含めるため、right = mid とし区間を狭める。
Copy link

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;
Copy link

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 に書くのは、まだいいですが。

Copy link
Owner Author

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) {
Copy link

Choose a reason for hiding this comment

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

const vector& と const つけてもいいかもしれません。

Copy link
Owner Author

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>
Copy link

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.

Copy link

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 は貼り付けません。)

Copy link
Owner Author

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) {
Copy link

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) {

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ずれるとバグるので、すこし不安な気持ちになりました

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.

6 participants