Skip to content

39. Combination Sum#52

Open
dxxsxsxkx wants to merge 1 commit into78_subsetsfrom
39_combination_sum
Open

39. Combination Sum#52
dxxsxsxkx wants to merge 1 commit into78_subsetsfrom
39_combination_sum

Conversation

@dxxsxsxkx
Copy link
Copy Markdown
Owner

return all_combinations;
}
private:
struct CombinationAndTargetAndStart {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

名前がいまいちな気がしますね。Personの代わりに、FirstNameAndLastNameAndAgeみたいな名前をつけるような印象です。CombinationContextとかSearchStateでどうでしょうか?

state.push({{}, target, 0});

while (!state.empty()) {
auto [combination, remaining, start] = state.top();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ここ、コピー走ってますね。

for (int i = start; i < candidates.size(); ++i) {
std::vector<int> next_combination = combination;
next_combination.push_back(candidates[i]);
state.push({next_combination, remaining - candidates[i], 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.

ここもコピーが走ってます。

Comment on lines +12 to +16
const std::vector<int>& candidates,
int target,
int start,
std::vector<int>& current_combination,
std::vector<std::vector<int>>& all_combinations
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

引数がやや多いように感じました。その内3個は毎回同じなので、lambdaを使うか、変わらない部分はstructでまとめるとかあたりですかね。

Dynamic programming でも解けるみたい。

この考えはなかった。この問題の制約上あんまり劇的には効率化できないが、発想としては大事だなあ。
> 先にcandidatesをsortしておけば、total+candidates[i] > target となったときにi以降の要素を見る必要はないのでサボれる。
Copy link
Copy Markdown

@huyfififi huyfififi Mar 22, 2026

Choose a reason for hiding this comment

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

322. Coin Change でも同じような最適化がありましたね。

return all_combinations;
}
private:
void findCombinations(
Copy link
Copy Markdown

@huyfififi huyfififi Mar 22, 2026

Choose a reason for hiding this comment

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

LeetCode の関数名と同じように命名していることかと思いますが、昔 Google C++ Style Guide を参照されていたような気がするので、それに従うなら PascalCase が良いですかね。

https://google.github.io/styleguide/cppguide.html#Function_Names

Ordinarily, functions follow PascalCase: start with a capital letter and have a capital letter for each new word.
AddTableEntry()
DeleteUrl()
OpenFileOrDie()

class Solution {
public:
std::vector<std::vector<int>> combinationSum(std::vector<int>& candidates, int target) {
std::vector<std::vector<int>> all_combinations;
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 は処理のためのバッファで呼び出し元は本来知らなくていいのに、呼び出し元で作成し渡してやらなくてはいけないのがちょっとモヤッとしました。

@mamo3gr
Copy link
Copy Markdown

mamo3gr commented Mar 23, 2026

コメントされている箇所以外は、概ね読みやすかったです。

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