121. Best Time to Buy and Sell Stock#20
Conversation
|
|
||
| for i, price in enumerate(prices): | ||
| if price < prices[buy_day]: | ||
| buy_day = i |
There was a problem hiding this comment.
このままでもほとんど問題ないと思いますが ，あえて挙げるなら買う"日" buy_day に"インデックス" i を代入するのがちょっとだけ気になりました．
buy_index などにすると買う日"のインデックス" になって両者が揃うと思います．
(追記)
今後 datetime.date 型の変数を使い始めそうなクラスのメソッドなら個人的に気になるんですが，今回の例なら気にしなくても良いと思います．
There was a problem hiding this comment.
私も...dayという変数にインデックスを入れているのは少し分かりづらい気がしました。
インデックスかビジネスロジックで必要な値かがわかるようにしたいです。
そういう意味で最初のbuy_day=0はループが始まる前に初期化しているので、値の初期値なのかインデックスの始まりなのかがこの時点ではわからないところに少し負荷があるように感じます。
インデックスであることが分かる名前であればこの後ループが出てくることが想像できて読みやすいです。
| class Solution: | ||
| def maxProfit(self, prices: List[int]) -> int: | ||
| max_profit = 0 | ||
| buying_price = prices[0] |
There was a problem hiding this comment.
問題の制約上そのような入力は Leet Code からは与えられませんが，開発現場でこのメソッドを実装するなら， prices として空のlistが入力されてもエラーが出ないように実装したいなと思いました．
def maxProfit(self, prices: List[int]) -> int:
if not prices:
return 0
max_profit = 0
buying_price = prices[0]
# (以下同じ)
| # Step 1 | ||
|
|
||
| - 変数を以下の通りにする | ||
| - stockを買う日: `buy_day` |
There was a problem hiding this comment.
個人的には最安値で買うという意味を持たせたいなと思いました。
pricesのなかのminを保持したいという意図が伝わりやすい気がしています。
|
|
||
| for i, price in enumerate(prices): | ||
| if price < prices[buy_day]: | ||
| buy_day = i |
There was a problem hiding this comment.
私も...dayという変数にインデックスを入れているのは少し分かりづらい気がしました。
インデックスかビジネスロジックで必要な値かがわかるようにしたいです。
そういう意味で最初のbuy_day=0はループが始まる前に初期化しているので、値の初期値なのかインデックスの始まりなのかがこの時点ではわからないところに少し負荷があるように感じます。
インデックスであることが分かる名前であればこの後ループが出てくることが想像できて読みやすいです。
| for i, price in enumerate(prices): | ||
| if price < prices[buy_day]: | ||
| buy_day = i | ||
| else: |
There was a problem hiding this comment.
step2,3でminを使った実装に置き換えられていて良いと思いました。
if-elseだと分岐をifの分岐条件を頭に入れたままelseが排他的条件の前提で読む必要があって少し認知不可が高い気がしています。
また、レビューでは境界値を考えたときに等価の場合はelse側に寄せるでよいのかなど余計なことを考える必要が出てくる気がするので、minのほうが不要な分岐を見せないことでレビュー観点もシンプルになる気がします。
3 participants
This problem: 121. Best Time to Buy and Sell Stock
Next problem: 35. Search Insert Position