Skip to content

Generate Parentheses#6

Merged
Exzrgs merged 5 commits intomainfrom
arai60-generate-parentheses
May 20, 2024
Merged

Generate Parentheses#6
Exzrgs merged 5 commits intomainfrom
arai60-generate-parentheses

Conversation

if len(bracket_stack) == bracket_limit * 2:
return bracket_stack

open_count = bracket_stack.count("(")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

毎回カウントしているため、時間計算量が n 倍増えてしまっていると思います。 open_count を引数で引き回してはいかがでしょうか?また、 close_count は open_count と blacket_limit から計算できると思います。

個人的には変数名は num_opens にすると思います。 open_count でも良いと思います。

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.

ご指摘ありがとうございます。
本当ですね...修正します🙏

open_count = bracket_stack.count("(")
close_count = bracket_stack.count(")")
if open_count < bracket_limit:
new_bracket_stack = list(bracket_stack)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

新しい list を毎回作っているため、ここでも時間計算量が n 倍増えてしまっているように思います。 append して、 append_bracket() から返ってきたときに pop() してはいかがでしょうか?

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.

こちらもご指摘ありがとうございます。
修正します🙏

class Solution:
def generateParenthesis(self, n: int) -> List[str]:
answer = []
def append_bracket(bracket_limit, bracket_stack):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bracket_limitは変化しないので、append_bracketのパラメータに含める必要はないです。

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.

たしかに、定数として扱ってよいということですね。
ありがとうございます。
自分に欠けていた感覚でした。

answer = []
def append_bracket(bracket_limit, bracket_stack):
if len(bracket_stack) == bracket_limit * 2:
return bracket_stack
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

関数名はgenerateParenthesisになっているので、bracketではなく、parenthesisに統一した方がいいでしょう。

class Solution:
def generateParenthesis(self, n: int) -> List[str]:
brackets_combinations = []
def append_bracket(parentheses, open_count):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

parenthesesも、brackets_combinationsと同じく、パラメータに含める必要はないです。inner functionを使うメリットの一つは外のスコープの変数にアクセスできることです。パラメータの数は一般に少ない方がいいです。

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.

ご指摘ありがとうございます。
open_countも同じように関数の外に書いても動作すると思いますが、そこを引数として渡す理由は、値渡しだからと考えてよいですか?
すなわち、parenthesesは参照渡しで他に影響を与えてしまうので、それなら引数として渡さないほうが良いという解釈でよろしいでしょうか?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

正直そこまで考えていなかったのですが、確かにopen_countも外でも大丈夫ですが、 毎回、open_count += 1open_count -= 1するのは冗長な気がします。 一方、parenthesesの場合は、append_bracketの1番目の引数は常にparenthesesなので、これもrepetitiveに感じました。

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.

なるほど、ありがとうございます。
感覚理解できました🙏

@@ -0,0 +1,27 @@
"""
nは定数だから引数に含めなくてよい
関数名がparenthesisだから変数名もそれに合わせたほうが良い
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

まだbracketという単語使っていますよね。brackets_combinations, append_bracket

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.

ほんとですね...
ありがとうございます🙏

close_count = len(parentheses) - open_count
if open_count < n:
parentheses.append("(")
append_bracket(parentheses, open_count+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.

open_count + 1

class Solution:
def generateParenthesis(self, n: int) -> List[str]:
brackets_combinations = []
def append_bracket(parentheses, open_count):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

正直そこまで考えていなかったのですが、確かにopen_countも外でも大丈夫ですが、 毎回、open_count += 1open_count -= 1するのは冗長な気がします。 一方、parenthesesの場合は、append_bracketの1番目の引数は常にparenthesesなので、これもrepetitiveに感じました。

parentheses_combinations.append("".join(parentheses))
return

close_count = len(parentheses) - open_count
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

変数に入れるのは肯定的です。場所は、必要となる場所の上くらいでもいいかも。

@Exzrgs Exzrgs merged commit 37fae2d into main May 20, 2024
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