Skip to content

【Arai60】52問目 39. Combination Sum#52

Merged
shining-ai merged 1 commit intomainfrom
review52
Jun 29, 2024
Merged

【Arai60】52問目 39. Combination Sum#52
shining-ai merged 1 commit intomainfrom
review52

Conversation

@shining-ai
Copy link
Copy Markdown
Owner

@@ -0,0 +1,17 @@
sys.setrecursionlimit(2000)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

これはどういう意図で追加していますか?どうして2000ですか?
(単なる興味です)

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.

参考にしていた書籍が2000でやってたので、意図は全く無いです。

sys.setrecursionlimit(2000)
class Solution:
def combinationSum(self, candidates: List[int], target: int) -> List[List[int]]:
def make_combinations(remain, combination, begin):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

なんとなくbeginの順番がもう少し左側な気がします。重要な引数ほど左にいると良いのかなと思っていてremain, begin, combinationの順番の方が良いかなと思います。個人的にはbegin, remain, combinationですが、ここまでいくと好みの問題かもしれないです。

重要な引数ほど左にいると良いというのは、Pythonなど言語によっては引数のデフォルト値も取れることも関係していると思います。

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.

重要な引数ほど左にいると良い

このことを知らなかったので、順番は意識してませんでした。
ありがとうございます。

if remain == 0:
combinations.append(combination[:])
return
for i in range(begin, len(candidates)):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

begin == len(candidates)のときにrangeで空のシーケンスが変えるので結果的に終了しますが、明示的にチェックして上げても良いかなと思います。

return
for i in range(begin, len(candidates)):
combination.append(candidates[i])
make_combinations(remain - candidates[i], combination, 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.

個人的には合計がtargetになるために必要な残りの値(remain)を管理するより、現在の合計を管理する方が素直かなと思いますが、好みの問題かもしれないです。

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.

現在の合計だと次に何が来ればいいのかすぐ分からず、結局target-現在値を考えるのかと思いremainにしました。
確かに素直な実装じゃない気がしてきました。

@shining-ai shining-ai merged commit 5c07c4e into main Jun 29, 2024
@shining-ai shining-ai deleted the review52 branch June 29, 2024 14:08
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.

2 participants