Skip to content

121. Best Time to Buy and Sell Stock #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fhiyo
Copy link
Owner

@fhiyo fhiyo commented Jul 7, 2024


```py
class Solution:
def maxProfit(self, prices: List[int]) -> int:

Choose a reason for hiding this comment

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

自分は前からpricesを見ていくほうが自然だと思います。あえてreverseして処理する必要性がないかなと思いました。この問題ではないですが、Stream処理的にpriceが1つずつ渡される、とかを考えると素直に前から見れるなら見るほうが良いかなと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

Stream処理的にpriceが1つずつ渡される、とかを考えると素直に前から見れるなら見るほうが良いかなと思います。

これたしかになと思いました。ありがとうございます。

Copy link

Choose a reason for hiding this comment

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

自分もあえてreverseする必要はないかなという意味で、hayahshi sanと同意見です!

@kzhra
Copy link

kzhra commented Jul 8, 2024

コードに関しては言うことがありません。いいと思います。
kadane のアルゴリズムでも(冗長ですが)解けなくはないので以下のおださんのやり取りを見ておくと、何か役立つかもしれません。
https://discord.com/channels/1084280443945353267/1206101582861697046/1208414507735453747


```py
class Solution:
def maxProfit(self, prices: List[int]) -> int:
Copy link

Choose a reason for hiding this comment

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

自分もあえてreverseする必要はないかなという意味で、hayahshi sanと同意見です!

Comment on lines +60 to +61
for i in range(len(prices)):
for j in range(i + 1, len(prices)):
Copy link

Choose a reason for hiding this comment

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

個人的には呼び出し回数が2回でも、最初にlength = len(prices)として、あとは変数を使いたいなという感覚はあります。

Choose a reason for hiding this comment

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

自分はどっちでも良いかなと思います。len(prices)とあった方が、prices変数のlength分ループを回しているというのが分かりやすいと思います。

Copy link

Choose a reason for hiding this comment

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

なるほどです!
len(prices)の方が処理がわかりやすいというは目からウロコでした。たしかにですね👀
小田さんも同意見みたいです。
https://github.com/NobukiFukui/Grind75-ProgrammingTraining/pull/34/files#r1660585965

自分も基本はどちらでも良いのですが、これくらいのスコープであればlengthでもわかりやすいかなというのと、len呼び出しのオーバーヘッドが気になるということ、あとはkou sanの以下コメントに近い考えもあるので、最初に初期化したいって感じです。
https://github.com/NobukiFukui/Grind75-ProgrammingTraining/pull/34/files#r1659881480

今回はここだけですが、近いコードを書くときに複数回len()を見たくなるケースが出てくるだろうという意味でも、ループ前に定義すると良いかと思います。

Copy link

Choose a reason for hiding this comment

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

向こうにコメント貼っていただいていたので、こちらでもコメントしておきます。

(元々の言い方はあまり良くなかったですが、)
loop中に呼び出す関数がどの程度コストがかかるかは気にしておいた方が良い、という趣旨でコメントしていました。
仮にDB呼び出しするとよく言われるN+1問題に遭遇するので、、、くらいです。
ですので本来の意図としては、len()が定数時間かはあまり関係はないです。

len(prices)の方が読みやすいは確かにそうですね。
len_prices = len(prices)と定義すると幾らかマシですが、ここがボトルネックになることはほとんどないので、
チーム内で決め事にすれば良いと思いました。

@erutako
教えていただいてありがとうございます。

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