Conversation
| def are_branckets_pair(open, close): | ||
| pair_dict = { | ||
| '}': '{', | ||
| ')': '(', | ||
| ']': '[' | ||
| } | ||
| if pair_dict[close] == open: | ||
| return True | ||
|
|
||
| return False |
There was a problem hiding this comment.
この場合だと、同じ二つの括弧を入れていたとしても、引数の順番によって異なる返り値が返ってくるため、順番に依存しないような関数にしたいと思いました。
たとえばこんな感じ？動作までは確認できていないので、もしかしたら動かないかもしれないです。
def are_branckets_pair(bracket1, bracket2):
valid_pairs = [
{'(', ')'},
{'[', ']'},
{'{', '}'}
]
# 集合は順序を持たないため { '(', ')' } も { ')', '(' } も同じ扱いのはず
return {bracket1, bracket2} in valid_pairs| # 以下はまとめて `return not open_branckets`とテクニカルに書けるがちょっとわかりにくい. | ||
| if len(open_branckets) != 0: | ||
| return False | ||
|
|
There was a problem hiding this comment.
個人的には return not open_branckets の方がわかりやすく感じます。趣味の範囲なのだろうなーと思います。
There was a problem hiding this comment.
私も条件式で返せるのであればreturn <条件式>のようにすると思いました。
| for s in s: | ||
| if is_close_brancket(s): |
There was a problem hiding this comment.
次のステップでは改善されていたためお気付きだと思いますが、s in s で同じ変数名を使うのが怖いなと感じました。これ以降の char in s の方が良さそうです。
| def is_close_brancket(s): | ||
| return s in ['}', ']', ')'] | ||
|
|
||
| def are_branckets_pair(open, close): |
There was a problem hiding this comment.
メソッドの命名について、boolを返すメソッドで are_* から始まるものを始めて見たので違和感を感じました。個人的にはis_pair_brackets,is_pairなどにすると思いました。
There was a problem hiding this comment.
自分も今まで見たことなかったのですが、英語的にはこうかなと考えて書きましたが、複数形に対してもisを使うのが普通のようですね
| if not open_branckets or open_to_close[open_branckets.pop()] != char: | ||
| return False |
There was a problem hiding this comment.
個人的には2つに条件式を分けたほうが読みやすいと感じました。not or notのように続くよりは一度に読む量を減らせるので認知負荷が下がるという感覚です。
| if not open_branckets or open_to_close[open_branckets.pop()] != char: | |
| return False | |
| if not open_branckets: | |
| return False | |
| if open_to_close[open_branckets.pop()] != char: | |
| return False``` |
t9a-dev
left a comment
t9a-dev
left a comment
There was a problem hiding this comment.
細かい点ですが全体的に bracketsがbrancketsになっている点が気になりました。
3 participants
解く問題
Valid Parentheses
次に解く問題
Reverse Linked List