Conversation
|
|
||
| for(const auto& price: prices) { | ||
| if(buying_price > price) { | ||
| // price - buying_price > 0 のとき |
There was a problem hiding this comment.
price - buying_price < 0 じゃないでしょうか？より安い価格で買えるのであれば更新するということですよね。
また、上のifの条件式を見ればすぐ分かる内容をコメントで書いても処理の流れが追いづらくなるだけになりそうかな、と思いました。
| int max_substract = 0; | ||
|
|
||
| for(int i = 0; i < prices.size() - 1; ++i) { | ||
| int current_max = 0; |
There was a problem hiding this comment.
この変数は不要で、max_substractだけでいいんじゃないかと思いました。
int max_substract = 0;
for (int i = 0; i < prices.size(); ++i) {
for (int j = i + 1; j < prices.size(); ++j) {
max_substract = max(max_substract, prices[j] - prices[i]);
}
}
return max_substract;
| int selling_price = 0; | ||
| for(int i = 1; i < prices.size(); ++i) { | ||
| if(prices[i] < buying_price) { | ||
| // prices[i] - buying_price < 0 のとき |
There was a problem hiding this comment.
if(prices[i] < buying_price)と同じ情報なのでコメントをつけなくて良いと思います。
|
|
||
| // kadane | ||
| // 変動価格の累積和を作る | ||
| vector<int> cums_of_daily{daily_price_change[0]}; // [0, -6, -2, -4, 1, -1], cumulative sum |
There was a problem hiding this comment.
これ、[0, -6, -2, -4, -1, -3]じゃないでしょうか？
また、変動価格の累積和を作るということであれば、
変動価格：[P(1)-P(1), P(2)-P(1), P(3)-P(2), ...P(N)-P(N-1)]
変動価格の累積和：[P(1)-P(1), P(2)-P(1), P(3)-P(1), ...P(N)-P(1)]
となるので、直接変動価格の累積和を計算する方が良いと思います。
|
レビューワーにとって何の問題に対するプルリクエストなのかが分かりやすいよう、プルリクエストのタイトルに問題名と番号を入れるとよいと思います。 |
|
|
||
| int buying_price = prices[0]; | ||
| int selling_price = 0; | ||
| for(int i = 1; i < prices.size(); ++i) { |
There was a problem hiding this comment.
for の後にスペースを空けることをお勧めいたします。
https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace
Space after the keyword in conditions and loops.
Google C++ Style Guide 等、有名なスタイルガイドを一通り読まれることをお勧めいたします。ただし、丸暗記したり、これらに完全に準拠してコードを書く必要はないと思います。ある程度参考にする程度でよいと思います。
| if (prices.size() < 2) return 0; | ||
|
|
||
| // 1日ごとの価格の変動を入れる配列 | ||
| vector<int> daily_price_change{0}; // [0, -6, 4, -2, 3, -2] |
There was a problem hiding this comment.
{ の前にスペースを空けることをお勧めいたします。
https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace
Open braces should always have a space before them.
| } | ||
|
|
||
| // データが降順に並んでいた場合の処理 | ||
| return max_subarray < 0 ? 0 : max_subarray; |
There was a problem hiding this comment.
自分なら max(0, max_subarray) と書くと思います。個人的に三項演算子は、読んでいてやや認知負荷が高いように感じます。
| int buying_price = prices[0]; | ||
| int profit = 0; | ||
|
|
||
| for(const auto& price: prices) { |
There was a problem hiding this comment.
変数の型のサイズが CPU のレジスタ幅に対して大きすぎない場合は、値で受け取ることをお勧めいたします。
for (auto price : prices) {
const auto& price と参照で受け取ると、 price の値にアクセスするときに参照を経由してアクセスすることになります。これはアセンブリや機械語のレベルでは、間接アドレッシングでアクセスすることになる場合があります。これにより直接アドレッシングにくらべて無駄な処理が起こる場合があります。
詳しくは
colorbox/leetcode#29 (comment)
をご覧ください。
4 participants
https://leetcode.com/problems/best-time-to-buy-and-sell-stock/description/