Skip to content

122. Best Time to Buy and Sell Stock 2#38

Open
dxxsxsxkx wants to merge 1 commit into121_best_time_to_buy_and_sell_stockfrom
122_best_time_to_buy_and_sell_stock_2
Open

122. Best Time to Buy and Sell Stock 2#38
dxxsxsxkx wants to merge 1 commit into121_best_time_to_buy_and_sell_stockfrom
122_best_time_to_buy_and_sell_stock_2

Conversation

@dxxsxsxkx
Copy link
Copy Markdown
Owner

}

int profit = 0;
for (int i = 1; i < prices.size(); i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i++と書くより++iと書いたほうがいいみたいなことをどこかで見たことがある気がします。
あんまり詳しくないんですがi++よりも少しパフォーマンスが良いみたいです。
ただ今回の場合だとコンパイラがC++を機械語に変換するときに最適化されて両方変わらないコードが出来上がるかもしれません。
google c++ style guideには基本的に++iを使えみたいなことが書いてありました。
https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

お二人ともありがとうございます。

class Solution {
public:
int maxProfit(std::vector<int>& prices) {
if (prices.empty()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

このif文はなくても動くのではないでしょうか?(試してないので間違ってたらすみません)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

あ、確かにそうですね。ありがとうございます。i の範囲と profit の初期化で対応できてました。

public:
std::vector<std::vector<int>> memo;

int _helper(std::vector<int>& prices, int current_time, bool is_holding) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

「ヘルパー」という名前にしては複雑な処理をしているので、より具体的な名前にしたいです。

return memo[current_time][is_holding];
}

int result;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

これは profit ですかね。

memo.assign(prices.size(), std::vector<int>(2, -1));
return _helper(prices, 0, false);
}
public:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: privateですかね。

return _helper(prices, 0, false);
}
public:
std::vector<std::vector<int>> memo;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

memoがどういう情報を持っているかコメントで補足するか、変数名を工夫すると良いと思いました。
bool型のis_holdingをsecond indexに入れているのに少し驚きましたが、一度理解できればわかりやすくて良い記法ですね。


[参照](https://github.com/olsen-blue/Arai60/pull/38/changes#r1980557395):状態遷移を入れるとき、行列にするよりも配列を2つ持つ方が良いかも?

- あと自分の書き方だと `-1` がマジックナンバーになってる
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

メンバ変数としてstatic constexpr int UNCALCULATED = -1などとするのも可読性や保守性の観点からアリだと思います。今回は意味が自明なので-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.

5 participants