Skip to content

Valid Parentheses#7

Open
bumbuboon wants to merge 1 commit intomainfrom
validParentheses
Open

Valid Parentheses#7
bumbuboon wants to merge 1 commit intomainfrom
validParentheses

Conversation

@bumbuboon
Copy link
Copy Markdown
Owner

Copy link
Copy Markdown

@hayashi-ay hayashi-ay left a comment

Choose a reason for hiding this comment

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

コメントしました。

Comment thread validParentheses.md
- ([)]のような事例に対応できていない。

```python
class Solution:
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.

ありがとうございます。手作業でやるならどうするかを考えたことはなかったので次から試してみたいと思います。

Comment thread validParentheses.md
collections.dequeを用いてみる

- [Listとdequeの計算量の違い](https://wiki.python.org/moin/TimeComplexity)
- append,popどちらもO(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.

定数倍の処理を無視していて、少し乱暴に思えます。dequeの方は双方向LinkedListを使っているため、単にStackとして使用するならListに比べてオーバーヘッドがあると思います。

Comment thread validParentheses.md
```

- stackに入れて後ろから確かめていくのは思いつかなかった
- `return not stack`で返すとbool値になるのは知らなかった
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 thread validParentheses.md
return not stack
```

いろいろ調べていたら4時間くらいかけてしまった。これで良いのだろうか?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LeetCode解くことがゴールではないのでいいのではないでしょうか。慣れれば調べるのに掛かる時間も減ると思います。

Comment thread validParentheses.md
if c in open_to_close:
stack.append(c)
continue
if not stack or c != open_to_close[stack.pop()]:
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の条件を分けて書いてしまいますね。あとstackの操作もif分とは別にやると思います。まあ好みかもしれないです。

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 thread validParentheses.md
if not stack or c != open_to_close[stack.pop()]:
return False

return not 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.

まあ選択肢の幅として。
return len(stack) == 0

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.

個人的には好みの範囲かなと思ってます。どの記法に慣れているかだったりPython以外の馴染みのある言語でも読みやすさも変わってきそうですし。個人的にはlen(stack) == 0の方が配列の要素が0であることを確認しているのが直感的に分かるのでlenを使う方が好みです。
大事なのは、色々書き方があることを知っているうえで自分が気に入っているのを使うというプロセスかなと思います。

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-8 と Google Style Guide は strings, lists, tuples は implicit でということでしたね。
趣味の範囲でしょう。大規模開発などでは周りを見て合わせましょう。
https://google.github.io/styleguide/pyguide.html#2144-decision
https://peps.python.org/pep-0008/#other-recommendations:~:text=For%20sequences%2C%20(strings%2C%20lists%2C%20tuples)%2C%20use%20the%20fact%20that%20empty%20sequences%20are%20false%3A

Comment thread validParentheses.md
collections.dequeを用いてみる

- [Listとdequeの計算量の違い](https://wiki.python.org/moin/TimeComplexity)
- append,popどちらもO(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.

計算量に興味がありすぎるかもしれません。計算量は計算時間を見積もるための手段です。だから、定数倍にも興味を持ちましょう。また、内部実装とできることの差異により注目しましょう。

Comment thread validParentheses.md
return not stack
```

いろいろ調べていたら4時間くらいかけてしまった。これで良いのだろうか?
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 thread validParentheses.md
"{": "}",
"[": "]"
}
open_brackets = LifoQueue()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

queue.LifoQueue() は内部でスレッド同期が取られるため、取られないデータ構造に比べて動作が遅いと思います。プロデューサー・コンシューマーパターンを実装するときなど特別な場合を除き、避けたほうがよいと思います。

Comment thread validParentheses.md

class Solution:
def isValid(self, s: str) -> bool:
open_to_close = {
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 thread validParentheses.md
class Solution:
def isValid(self, s: str) -> bool:
stack = []
pair = {')':'(','}':'{',']':'['}
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.

承知しました。今度からは気をつけたいと思います。

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.

5 participants