Conversation
Yuto729
changed the title
| current = head | ||
| while current is not None: | ||
|
|
||
| # 重複しないケースを先に処理してcontinue | ||
| if current.next is None or current.next.val != current.val: |
There was a problem hiding this comment.
ここの改行が気になりました。改行を入れるのであれば、変数定義とwhile文の間に入れると思いました。
| current = head | |
| while current is not None: | |
| # 重複しないケースを先に処理してcontinue | |
| if current.next is None or current.next.val != current.val: | |
| current = head | |
| while current is not None: | |
| # 重複しないケースを先に処理してcontinue | |
| if current.next is None or current.next.val != current.val: |
| current = deleteCurrentDuplicates(current) | ||
|
|
||
| previous.next = current | ||
| continue |
There was a problem hiding this comment.
ここの改行が気になりました。個人的にはここに改行は入れないと思いました。
| current = deleteCurrentDuplicates(current) | |
| previous.next = current | |
| continue | |
| current = deleteCurrentDuplicates(current) | |
| previous.next = current | |
| continue |
t9a-dev
left a comment
t9a-dev
left a comment
There was a problem hiding this comment.
細かい点ですが改行の位置のみ気になる点がありました。
全体的には読みやすいと思いました。
Yuto729
changed the title
| if current.next is not None and current.next.val == current.val: | ||
| while current.next is not None and current.next.val == current.val: | ||
| current = current.next | ||
|
|
There was a problem hiding this comment.
if A:
if B:
長めの処理
continue
短めの処理のような書き方は、
if A:
if not B:
短めの処理
continue
長めの処理という順序にした方がわかりやすいかなと感じます。
具体的な例で言うと、例えば何かの機械の説明書を読むときに、「このマークが出ていたらすぐに停止してください。出ていない正常な場合は、この処理をして〜、それから〜をしてください。」か「正常な場合は、この処理をして〜、それから〜をしてください。正常でなく、このマークが出ていたらすぐに停止してください。」のどちらが読みやすいか考えると後者の方が読みやすい、のようなイメージだと自分では理解しています。
今回の場合で書くとしたらこんな感じですかね。
class Solution:
def deleteDuplicates(self, head: Optional[ListNode]) -> Optional[ListNode]:
dummy = ListNode()
dummy.next = head
previous = dummy
current = head
while current is not None:
if current.next is None or current.val != current.next.val:
previous = current
current = current.next
continue
value_to_delete = current.val
while current is not None and current.val == value_to_delete:
current = current.next
previous.next = current
return dummy.next| if current.next is not None and current.next.val == current.val: | ||
| current = deleteCurrentDuplicates(current) |
There was a problem hiding this comment.
関数化すると、たとえ複雑な処理であっても結局何がしたいのかがすぐにわかるようになるので、こう書いてもらえると個人的には読みやすくて嬉しいですね。
また、条件文の左に主役がいる方が私は読みやすいなと感じます。どちらも趣味の範囲な気がするので参考程度で。
if current.next is not None and current.val == current.next.val:
current = deleteCurrentDuplicates(current)
| def deleteDuplicates(self, head: Optional[ListNode]) -> Optional[ListNode]: | ||
| dummy = ListNode() | ||
| dummy.next = head | ||
| previous = dummy |
There was a problem hiding this comment.
自分が指摘をもらった内容ですが、last_confirmed などのほうがより変数の意味が分かりやすく感じると思いました。
https://github.com/resumit30minutes/leetcode-arai60-practice/pull/4/files#r2497684969
4 participants
解く問題
Remove Duplicates From Sorted List II
次に解く問題
Valid Parentheses