Skip to content

Create 39. Combination Sum.md#1

Merged
Mike0121 merged 2 commits intomainfrom
Mike0121-39.-Combination-Sum
Nov 1, 2025
Merged

Create 39. Combination Sum.md#1
Mike0121 merged 2 commits intomainfrom
Mike0121-39.-Combination-Sum

Conversation

@Mike0121
Copy link
Copy Markdown
Owner

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

current_combinationは、all_valid_combinationのように外に置けます。一般にパラメータの数は少ない方がいいです。

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.

可能な限り関数化してしまった方が良いかと勝手に思っていたので、覚えておきます。

一般に関数のパラメータの数は少ない方がいい

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

関数化した方がいいのはその通りなのですが、パラメータの数についての指摘でした。この関数ならindex, current_target, current_combinationの3個ありますが、current_combinationはいつも同じです。def find_vaild_combination(index, current_target, current_combination):

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.

なるほど、理解できました。有難うございます。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

変化のある方を比較演算子の左側に置く方がわかりやすいかもしれません。index == len(candidates)

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.

ありがとうございます。こちらはリーダブルコードでも同様の指摘があった記憶があり、修正しておきます。

変化のある方を比較演算子の左側に置く方がわかりやすい

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ifの後のスペースは一つにしましょう。

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.

ありがとうございます、見落としていました。

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が1,000,000で、candidatesが[1, 2]だったらどうですか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

時間計算量も問題ありそうですね。

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.

ご指摘ありがとうございます。
自信がないので、改めてアドバイスをもとに考えを書きますので、再度確認していただけたら嬉しいです。

最悪空間計算量は、O(target/min(candidates))でしょうか?
長さ1,000,000の[1, 1, 1, 1, ....1]のリストになるので、この場合、O(1,000,000)になります。
また、targetが13、candidates が例えば、[2, 100]の場合、最悪空間計算量は長さ[2, 2, 2, 2, 2, 2]のリストを試す必要があるので、このようになると考えました。

最悪時間計算量は、現状こんがらがっています。

Copy link
Copy Markdown

@liquo-rice liquo-rice Apr 24, 2024

Choose a reason for hiding this comment

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

答えの数がどれだけかは分からないのですが、答えを保存するスペースを無視すると空間計算量はそれで合ってると思います。

時間計算量については、探索空間をN-ary木で考えると分かるかと思います。詳細はhttps://leetcode.com/problems/combination-sum/editorial を参照。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

答えの数ですが、candidates = [1..target] の場合、これは分割数というものですね。

分割数の極限は、ハーディとラマヌジャンが求めています。
a(n) ~ 1/(4nsqrt(3)) * e^(Pi * sqrt(2n/3)) as n -> infinity (Hardy and Ramanujan)
https://oeis.org/A000041
だいたい、13^sqrt(n) / 7n くらいですか。

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.

ありがとうございます。
時間計算量をコードに沿った二分木で考えていたので複雑になってしまったかと思います。
選択肢がcandidate数分(N)あり、最大の木の深さがtarget/min(candidates)のため、最大で計算は、根ノード分を考慮しN^(target/min(candidates) + 1)繰り返されるイメージです。

答えの数の分割数も少し調べて見ました。色々な求め方(表現形式)が合って面白かったです。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

複数形の方がいいかもしれないです。

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.

はい、修正いたします。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PEPは未確認ですが、演算子の周りにスペースを入れるのが一般的ではないでしょうか?index + 1current_target - candidates[index]

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.

PEPを確認して、ご指摘の通りでした。
今週中に忘れないうちにPEPに目を通しておきます。
代入(や他の)演算子を揃えるために、演算子の周囲に1つ以上のスペースを入れる
ソース: https://pep8-ja.readthedocs.io/ja/latest/#id9

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

```pythonと書くとsyntax highlightingがつきます

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

typo: find_all_combiantions -> find_all_combinations

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.

ありがとうございます、これはダメですね。以後、より厳密にtypoの見直しを気をつけます。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

使っていないです。

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.

ありがとうございます、これも気をつけます。。。

@Mike0121 Mike0121 changed the title 39. Combination Sum レビュー依頼 Create 39. Combination.md Nov 1, 2025
@Mike0121 Mike0121 changed the title Create 39. Combination.md Create 39. Combination Sum.md Nov 1, 2025
@Mike0121 Mike0121 merged commit 8627cc2 into main Nov 1, 2025
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.

3 participants