-
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?
Conversation
lc929.md
Outdated
- "."を使っている時と, "+"を使っている時。また, local nameかどうかを判断しないと望みの結果は得られない | ||
- 冗長そうな実装が出来上がったが、変数名が長いから読みにくいだけな気もする | ||
|
||
'''python |
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のstep1もコードブロックになっていなかったので、テンプレートか何かの段階で` が ' になっているかもしれません。
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.
ご指摘ありがとうございます、修正しました
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 comment
The 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 comment
The 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 comment
The 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, ...} になっていませんか?これは想定通りの挙動でしょうか。
この処理はおかしい気がしていて、ある domain に対して最後に確認した local (文字列) (もしくはこれまでに見たlocalの文字列の結合)に対して in の確認をしているので、入力が ["[email protected]", "[email protected]"] の時に 1 が return されませんか。
# 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 comment
The reason will be displayed to describe this comment to others. Learn more.
あ、Python のライブラリーを時々読むことをおすすめします。
https://docs.python.org/3/library/stdtypes.html#textseq
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 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を使った実装を見て同じことを思いました。
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 comment
The reason will be displayed to describe this comment to others. Learn more.
英単語から文字を削って変数名とすると、読み手に取って認知負荷が上がる場合があります。原則避けたほうがよいと思います。
参考までにスタイルガイドへのリンクを貼ります。
https://google.github.io/styleguide/pyguide.html#316-naming
Avoid abbreviation. In particular, do not use abbreviations that are ambiguous or unfamiliar to readers outside your project, and do not abbreviate by deleting letters within a word.
ただし、上記のスタイルガイドは唯一絶対のルールではなく、複数あるスタイルガイドの一つに過ぎないということを念頭に置くことをお勧めします。
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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
単語事態あまり聞いたことがなかったのですが調べて見たらcomputer scienceの文脈では使われているようですね. wikipediaで調べたら出てきました
|
||
- 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 comment
The reason will be displayed to describe this comment to others. Learn more.
RFC は規格ではないと思います。インターネット技術の標準的な仕様を記した文書のようです。
https://business.ntt-west.co.jp/glossary/words-00220.html
RFC(Request for Comments)とは、「コメントを求める」という意味で、インターネット技術の標準的な仕様を記した文書であり、技術情報や運用規則などが定められています。
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 comment
The reason will be displayed to describe this comment to others. Learn more.
+
が複数回出現する文字列の連結は、 f 文字列で書いたほうが読みやすい場合があります。
f"{local}@{domain}"
今回の場合は +
で十分読みやすいと思います。
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.
書き方でいうと, str.format() のような書き方があると思うのですがこちらは, str.format()は知識として知っている程度でよく、実装にはあまり使わない
くらいの認識で良いのでしょうか?
unique_emails.add("{}@{}".format(local, domain))
簡単に調べる限りですがあまり利点が見つからず, かつ読みにくいなとずっと思っていたのでお聞きしたいです。
(chatgpt)
- python2/3両方に対応している
- 複雑なフォーマットに柔軟と回答されました
pythonのドキュメント
- (パッと見ですが)フォーマットに柔軟
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.
個人的な感覚となりますが、その認識で十分だと思います。 Python 3.6 で f 文字列が導入されてからは、そちらを優先的に使うように思います。
No description provided.