Skip to content

【Arai60】13問目 349_Intersection of Two Arrays#13

Merged
shining-ai merged 3 commits intomainfrom
review13
Feb 19, 2024
Merged

【Arai60】13問目 349_Intersection of Two Arrays#13
shining-ai merged 3 commits intomainfrom
review13

Conversation

@shining-ai
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.

全般的に良いと思います。

def intersection(self, nums1: List[int], nums2: List[int]) -> List[int]:
nums1_set = set(nums1)
intersection = []
for num in nums1_set:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

変数名をnumの代わりにnum1とかにした方がnumという一般的な変数がnums由来ではなくnums1由来であることを別途覚えておく必要がなく読みやすい気がしました。

intersection = []
i = 0
j = 0
while i < len(nums1) and j < len(nums2):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

whileの中は素直にif elif elseで良いと思います。例外が2ケースと本命が1ケースあるというより同じレベルの処理が3ケースあるみたいな感じだと思います。

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.

綺麗に3ケースに分かれるので、その書き方のほうがいいかもしれませんね。
自然言語で説明する場合は、そのようにするので流れとしては自然ですね。

@shining-ai
Copy link
Copy Markdown
Owner Author

shining-ai commented Feb 18, 2024

Odaさんのコメントを転記
in list は、list を全部確認するので、O(n) の時間がかかります。

if num in nums2:

if nums1_sorted[i] not in intersection:

このあたりです。

特に後ろの、not in は何をやっているのか分からなくなる印象を受けるので、ちょっと作り直してみましょう。

私は、この場合は、結構強く if continue を勧めます。

if A:
  B
elif C:
  D
else E:
  F
G

という形は、B を読んだ後に、G があるかどうかを確認させているからです。

一般に、early return pattern などといいます。

if nums1_sorted[i] == nums2_sorted[j]:

これは書きません。
やりたかったら、コメントをつけるか、assert を書くでしょう。

ただ、nan というのがあって、nan == nan が False です。
IEEE754 を確認しておいてください。

@shining-ai
Copy link
Copy Markdown
Owner Author

Odaさんのコメントを転記
nums1_sorted[i] と intersection[-1] 一緒だと思うんですが、これ、普通に変数名をつけてしまいませんか。重要なやつには名前をつけましょう。関数への切り出しなども重要なやつに名前をつけるの一種ですね。

@shining-ai shining-ai merged commit 548a273 into main Feb 19, 2024
@shining-ai shining-ai deleted the review13 branch February 19, 2024 19:29
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.

2 participants