121. Best Time to Buy and Sell Stock (from Grind 75 questions Easy-Array)#3
121. Best Time to Buy and Sell Stock (from Grind 75 questions Easy-Array)#3
Conversation
| 12分 | ||
|
|
||
| ### 感想 | ||
| - minだけだとわかりにくいからminBuyingPriceとかにしたけど、長い？ |
There was a problem hiding this comment.
語彙的に minBuyingPriceがベストかどうかはわからないですが、わかりやすいならこの程度の長さは業務上問題ない認識です。
There was a problem hiding this comment.
良かったです！
僕もそのような感覚なのですが、leetcodeのsolutionsを見ていると皆さんかなり短い変数名ばかり使っておられるので、どうかなと感じた次第ですmm
Atcoderでもワンライナーにしがちな傾向があったりするので、このあたりはleetcode特有の傾向かもですねmm
| if (prices[i] < minPrice) { | ||
| minPrice = prices[i]; | ||
| } else { | ||
| maxProfit = Math.max(maxProfit, prices[i] - minPrice); | ||
| } |
There was a problem hiding this comment.
Early returnした方が読む側として認識するスコープを狭くできるので、elseは可能な限り避けた方がおすすめです。
| if (prices[i] < minPrice) { | |
| minPrice = prices[i]; | |
| } else { | |
| maxProfit = Math.max(maxProfit, prices[i] - minPrice); | |
| } | |
| if (prices[i] < minPrice) { | |
| minPrice = prices[i]; | |
| continue; | |
| } | |
| maxProfit = Math.max(maxProfit, prices[i] - minPrice); |
There was a problem hiding this comment.
[nits]
- あと
continueなくても結果は変わらない気がします。- 最小値が更新された後に、
prices[i] - minPriceが必ず0になるので、maxProfitに影響しないはずです。 - そうするとMath.minも使えるようになって、コードがもう少し簡潔になります。
- 最小値が更新された後に、
There was a problem hiding this comment.
コメントありがとうございます！
たしかにそうですね！mm
最小値が更新された後に、prices[i] - minPriceが必ず0になるので、maxProfit に影響しないはずです。
個人的には最小値が更新されたときは最大利益をもたらさないことが明らかなのに、解に影響はないとはいえ最大利益の比較をするというのが、あまり好みではなかったですmm
ただleetcodeのsolutionsを見ているとrossy0213sanの書き方が5割は超えていそうですので、そのほうが伝わりやすそうですねmm
There was a problem hiding this comment.
最初から機械的に以下の2つの作業をループしようとしていると考えると不思議と違和感がなくなりましたmm
- 今までの最小値と現在の値を比較して最小値を更新する
- 今までの最大利益と現在の最大利益を比較して最大利益を更新する
ifで書いてしまっていましたが、Math.min, maxを毎ループ呼び出すと考えるとif-elseで書かないことも違和感ないですし、読みやすくなるなと自己完結しましたmm
There was a problem hiding this comment.
個人的には最小値が更新されたときは最大利益をもたらさないことが明らかなのに、解に影響はないとはいえ最大利益の比較をするというのが、あまり好みではなかったですmm
気持ちわかります。[nits]つけてた理由がそこで、そうした方がいいという強い気持ちはないです。
この程度の意味のない処理をどうするかのコードについては好みの問題の気がします。
ただleetcodeのsolutionsを見ているとrossy0213sanの書き方が5割は超えていそうですので、そのほうが伝わりやすそうですねmm
それはもしかしたら単純にSolutions同士で参考しあってる可能性があります。また、leetcodeなどでは短い方がいいっていう判断方針に走ることが(よく)たまに見受けられます。もう一つのコメントにあるワンライナーの話もそうですが、業務上では読み手にとっていいかどうかが全てなので、短い方が絶対にいいということはないです。
今のコードはelseからcontinueに変更さえすれば、自分は業務上ではapprovalしていたと思います。
There was a problem hiding this comment.
同じような感覚の方がいらっしゃるということで少し安心しました！
ありがとうございます！
気持ちわかります。[nits]つけてた理由がそこで、そうした方がいいという強い気持ちはないです。
そういった可能性ありそうですね。
rossyさんのapprovalの基準も参考になりましたmm
それはもしかしたら単純にSolutions同士で参考しあってる可能性があります。
| ```Java | ||
| class Solution { | ||
| public int maxProfit(int[] prices) { | ||
| int minPrice = prices[0]; |
There was a problem hiding this comment.
空の配列が渡された場合もケアしておいたほうが良いかと思いました。
・minPrice の初期値を無限にしてループを0から開始する
・空の配列を例外処理する
問題の制約上テストケースに含まれないので、気にし過ぎかもしれませんが。
There was a problem hiding this comment.
空配列の考慮、確かに実務だと考えなくてはならない観点だと思いましたmm
個人的には色々調べて、空配列のような例外的な引数がよく現れるのでは例外処理、ほぼ想定できないのであればnullにして、起きる頻度に応じて呼び出し側の負担を調整してあげるのが良さそうだと感じましたmm
| int minPrice = prices[0]; | ||
| int maxProfit = 0; | ||
|
|
||
| for (int i = 1; i < prices.length; i++) { |
There was a problem hiding this comment.
https://github.com/erutako/leetcode/pull/3/files#r1628045156
でのコメントと一部被りますが、
1開始のループよりも0開始としたほうが読みやすいかもしれません。
また、その場合
for(int price: prices){
とするとさらに読みやすくなりそうです
There was a problem hiding this comment.
ありがとうございます！
インデックスを利用しないループは拡張for文で良いですね！
ループも0開始のほうがわかりやすいというのは自分も思いましたmm
| public int maxProfit(int[] prices) { | ||
| int min = prices[0]; | ||
| int maxProfit = 0; |
There was a problem hiding this comment.
メソッド名と変数が同じ規則で命名されているのが気になりました。
java では問題ないのでしょうか？
メソッドと変数名が同じなのは避けたほうがいいと思いました。
There was a problem hiding this comment.
個人的には今までの職場では特に気にしたことはなかったですね👀
この関数名はleetcodeのデフォルトの関数名を利用しているため、自分が実務で書くならcalculateMaxProfitなどのように動詞をつけそうです！
java では問題ないのでしょうか？
| int profit = prices[i] - min; | ||
| if (profit > maxProfit) { | ||
| maxProfit = profit; |
There was a problem hiding this comment.
Math.maxがあるので、そちらのほうがわかりやすそうですね。
ありがとうございます！
erutako
changed the title
erutako
changed the title
erutako
changed the title
5 participants
Question
https://leetcode.com/problems/best-time-to-buy-and-sell-stock/description/