Skip to content

Conversation

combination_value_sum = 0
target_combinations = []
def count_target_combinations(index):
nonlocal combination_value_sum
Copy link

Choose a reason for hiding this comment

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

step2 のように combination_value_sum を関数の引数として渡したほうがきれいに見えました。

また、変数名は sum_of_ の省略形の sum_ で始めるため、 sum_combination_values のほうが親善だと感じました。

return target_combinations

# stackでも書いてみる
# 実際再帰の深さによってどのようなデメリットがあるのかとかをちゃんと把握できていない気がする
Copy link

Choose a reason for hiding this comment

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

再帰の深さは、 CPython 3.11.9 のデフォルトだと 1000 と設定されています。これを超えると例外が飛びます。

再帰の深さの上限は取得・設定できます。
https://docs.python.org/ja/3/library/sys.html#sys.getrecursionlimit
https://docs.python.org/ja/3/library/sys.html#sys.setrecursionlimit

Copy link
Owner Author

Choose a reason for hiding this comment

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

こういう場合って、メモリを食べちゃうからスタックで実装したほうが良いんですかね?
それとも、最近のコンピュータだとメモリはそこまで気にしなくてもよいんですかね
nodchipさんならどうしますか?

def combinationSum(self, candidates: List[int], target: int) -> List[List[int]]:
target_combinations = []
combination_stack = [([], 0, 0)]
while combination_stack:

Choose a reason for hiding this comment

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

変数名 "combination_stack"に関してですが、型名よりもcombination以外の値も入ってることを変数名に反映した方が良いのかなと思いました。3つ変数が入ると少し難しいですね。combination_sum_indexとかだとちょっと微妙ですね。良い案思いつかずすみません。

Copy link
Owner Author

Choose a reason for hiding this comment

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

コメントありがとうございます。
たしかに、自分もそう思います。
この場合、無理になにか表現せず、単にstackとかでもよいような気がしてきました
もしくは、current_comb_info_stackとかですかね

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.

4 participants