Conversation
| ```py | ||
| class Solution: | ||
| def minSubArrayLen(self, target: int, nums: List[int]) -> int: | ||
| cumsum = [0] |
There was a problem hiding this comment.
リストなので、cumsums の方が分かりやすいのかなと思いました。
(cumsum にした方がいい理由が思い当たりませんでした。)
| if subarray_sum >= target: | ||
| min_length = min(min_length, end - begin) | ||
| subarray_sum -= nums[begin] |
There was a problem hiding this comment.
end == len(nums) の場合に begin を回すための処理かなと思いました。
だとすると、subarray_sum < target してたらもう subarray_sum は小さくなる一方だと思うので、もう引かなくてもいいのかなと思いました。
if subarray_sum < target:
break
min_length = min(min_length, end - begin)
subarray_sum -= nums[begin]ただ、begin のループを抜けようとしたらもうひと手間いると思うので、少し流れが違うだけですね。
| if min_length == len(nums) + 1: | ||
| return 0 |
There was a problem hiding this comment.
このケースは解が存在していないケースかと思います。この問題における解なしというのは sum(nums) < target のケースなので、この関数の一番始めでチェックしておけばそれ以降は解が存在する前提になり複雑度が減りそうです。
またここで判断するケースでも len(nums) + 1 が解なしを意味するというのはぱっと見ではわかりにくいので、math.inf など明らかにおかしいもので初期化しておくとよさそうに思いました。(上記に書いたように解が存在していることをチェックした後であれば min_length = len(nums) のような解となりうる値の中の最大値で初期化するとかもいいかと思います)
There was a problem hiding this comment.
math.inf, 型がfloatなのがちょっと気になるんですよね...使うメリットはあるので選択肢としてありだとは思いますが。
sum(nums) < target かを先にチェックするのは、それも手だと思いますがそこでO(n)かかるコストを払うトレードオフをどう見るかですね... (cpythonなので) C実装のsumは速いからいいだろうという考えもまた分かりますが。
There was a problem hiding this comment.
math.inf, 型がfloatなのがちょっと気になるんですよね...使うメリットはあるので選択肢としてありだとは思いますが。
なるほど、たしかに型が異なるのは気になるポイントかもですね。
sum(nums) < target かを先にチェックするのは、それも手だと思いますがそこでO(n)かかるコストを払うトレードオフをどう見るかですね... (cpythonなので) C実装のsumは速いからいいだろうという考えもまた分かりますが。
全体の時間計算量 O(n) を悪化させる修正ではないので、個人的には可読性を優先するかなというコメントでした。(書かれているとおりトレードオフなので、可読性の向上があまりないと感じていたり、少しでも計算を早めたいという状況であれば、選択は変わると思います)
| for end in range(len(cumsum)): | ||
| while cumsum[end] - cumsum[begin] >= target: | ||
| min_length = min(min_length, end - begin) | ||
| begin += 1 |
There was a problem hiding this comment.
個人的に二重ループになっていると少し複雑かなと思いました。if 文を使って left と right を順に進めていくようにするとループ自体は1つなのでわかりやすいかもです。またリーダブルコードという本には begin と end は 開閉区間([begin, end))の場合に用いるというような記述があったので、誤解を生む可能性があるかもです。(ここは所属組織の文化や慣習が大きいと思いますが)
| for end in range(len(cumsum)): | |
| while cumsum[end] - cumsum[begin] >= target: | |
| min_length = min(min_length, end - begin) | |
| begin += 1 | |
| left = 0 | |
| right = 1 | |
| while right < len(cumsum): | |
| if cumsum[right] - cumsum[left] < target: | |
| right += 1 | |
| else: | |
| min_size = min(min_size, right - left) | |
| left += 1 |
|
良いと思います。 |
| def minSubArrayLen(self, target: int, nums: List[int]) -> int: | ||
| cumsum = [0] | ||
| for num in nums: | ||
| cumsum.append(cumsum[-1] + num) |
There was a problem hiding this comment.
累積和を求めるときのこの書き方良いですね、参考にします。
自分が書くときのcumsum[i + 1] = nums[i] + cumsum[i]より見やすいと思いました。
| cumsum.append(cumsum[-1] + num) | ||
| min_length = len(nums) + 1 | ||
| for i in range(len(cumsum)): | ||
| j = bisect_left(cumsum, cumsum[i] + target) |
There was a problem hiding this comment.
i, j はもう少し変数名こだわっても良いかと思いました。
i を start_index (or start), jがend_index (or end)とかですかね?(ちょっと自信ないですが。)
| assert target > 0 | ||
| min_length = len(nums) + 1 | ||
| subarray_sum = 0 | ||
| left = -1 # exclusive |
There was a problem hiding this comment.
感想: indexに-1設定できると左の開区間を表現しやすくていいですね
(普段書いているRustだとこれがとってもやりにくい)
https://leetcode.com/problems/minimum-size-subarray-sum/description/