-
Notifications
You must be signed in to change notification settings - Fork 0
929. Unique Email Addresses #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
# step1 | ||
思考ログ | ||
- 条件分岐をいくつかするしかなさそう? | ||
- "."を使っている時と, "+"を使っている時。また, local nameかどうかを判断しないと望みの結果は得られない | ||
- 冗長そうな実装が出来上がったが、変数名が長いから読みにくいだけな気もする | ||
|
||
```python | ||
class Solution: | ||
def numUniqueEmails(self, emails: List[str]) -> int: | ||
domain_name_to_local_name = defaultdict(list) | ||
receive_mails_num = 0 | ||
|
||
for email in emails: | ||
at_idx = [idx for idx, s in enumerate(email) if s == "@"][0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 英単語から文字を削って変数名とすると、読み手に取って認知負荷が上がる場合があります。原則避けたほうがよいと思います。 参考までにスタイルガイドへのリンクを貼ります。 https://google.github.io/styleguide/pyguide.html#316-naming
ただし、上記のスタイルガイドは唯一絶対のルールではなく、複数あるスタイルガイドの一つに過ぎないということを念頭に置くことをお勧めします。 |
||
original_local_name = email[:at_idx] | ||
domain_name = email[at_idx+1:] | ||
|
||
processed_local_name = "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 比較のために正規化された local name という意味合いで、 canonicalized_local_name という変数名でもよいと思います。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 単語事態あまり聞いたことがなかったのですが調べて見たらcomputer scienceの文脈では使われているようですね. wikipediaで調べたら出てきました |
||
for s in original_local_name: | ||
if s == "+": | ||
break | ||
if s == ".": | ||
continue | ||
processed_local_name += s | ||
|
||
if domain_name in domain_name_to_local_name: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 以下の処理が想定通り動いてない気がします。defaultdict(list) であるdomain_name_to_local_name の value として str を設定したり、足したりしていませんか? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 想定通り動いていないというのは, LeetCodeで出てきたテストケース以外だと動かないのではないかということで合っていますかね? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 10行目と26, 27行目を読んで、domain_name_to_local_name は {domain: [local1, local2, ...], ...} のようになるのかなと想像しましたが、実際には {domain: local2, ...} になっていませんか?これは想定通りの挙動でしょうか。 |
||
if processed_local_name not in domain_name_to_local_name[domain_name]: | ||
domain_name_to_local_name[domain_name] += processed_local_name | ||
receive_mails_num += 1 | ||
else: | ||
domain_name_to_local_name[domain_name] = processed_local_name | ||
receive_mails_num += 1 | ||
|
||
return receive_mails_num | ||
``` | ||
|
||
# step2 | ||
参考にした方のレビューを貼る | ||
- https://github.com/potrue/leetcode/pull/14/files?short_path=6a8efe5#diff-6a8efe56493bdbfeb045b7aa123660406ebdd5dd4f0a011935562d4eefb9ab46 | ||
- step1で書いた実装を全てスマートに解いている上位互換のような実装だった. 確かにsplitで分け, replaceで置換しset()でuniqueに取れば良い話である | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. あ、Python のライブラリーを時々読むことをおすすめします。 |
||
- 問題を解く前の段階では, "異常な入力も想定しないと"と考えていましたが,実際に解いている時は完全に忘れていました. 意識がまだできていませんね。 | ||
- 今回の場合だと"@"が複数ある場合や, そもそもない場合などが想定できそうです | ||
|
||
- https://github.com/nktr-cp/leetcode/pull/15/files?short_path=0c860cd#diff-0c860cd754249868513e4f9054206317fa33d0f548fc3896ac2b3e11822fd852 | ||
- c++でも結局やっていることは同じであることが確認できた | ||
- RFCという規格があるそうだ. 初めて知った | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RFC は規格ではないと思います。インターネット技術の標準的な仕様を記した文書のようです。 https://business.ntt-west.co.jp/glossary/words-00220.html
|
||
|
||
- https://github.com/Satorien/LeetCode/pull/14/files | ||
- splitではなく, [find](https://github.com/Satorien/LeetCode/pull/14/files)でやる方法. step1においてidxをfor分で毎回探していたのを一発でやっている. [cpython](https://github.com/python/cpython/blob/e3dda8f81832008adf19906004f0cd53de95dd0b/Objects/unicodeobject.c#L1095)で実装を見ようと思ったが全くわからなかった(そもそもここであっているのかも微妙) | ||
|
||
- https://github.com/tokuhirat/LeetCode/pull/14/files?short_path=6ffc399#diff-6ffc399bc049f5459f86164c71308927e56dd61d7b4810e32a44b4a97c7b2c1b | ||
- エラーハンドリングが網羅的にされていた, 実務だとこれくらい必要なんだろうか | ||
|
||
# step3 | ||
```python | ||
class Solution: | ||
def numUniqueEmails(self, emails: List[str]) -> int: | ||
unique_emails = set() | ||
for email in emails: | ||
local, domain = email.split("@") | ||
local = local.split("+", 1)[0] | ||
local = local.replace(".", "") | ||
unique_emails.add(local + "@" + domain) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 書き方でいうと, str.format() のような書き方があると思うのですがこちらは,
簡単に調べる限りですがあまり利点が見つからず, かつ読みにくいなとずっと思っていたのでお聞きしたいです。 (chatgpt)
pythonのドキュメント
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 個人的な感覚となりますが、その認識で十分だと思います。 Python 3.6 で f 文字列が導入されてからは、そちらを優先的に使うように思います。 |
||
|
||
return len(unique_emails) | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at_idx = email.find("@")
のほうが読みやすいと思います。There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
そうですね, 他の方のPRを見てfindを使った実装を見て同じことを思いました。