Skip to content

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

929. Unique Email Addresses #13

wants to merge 2 commits into from

Conversation

rinost081
Copy link
Owner

No description provided.

lc929.md Outdated
- "."を使っている時と, "+"を使っている時。また, local nameかどうかを判断しないと望みの結果は得られない
- 冗長そうな実装が出来上がったが、変数名が長いから読みにくいだけな気もする

'''python

Choose a reason for hiding this comment

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

他のPRのstep1もコードブロックになっていなかったので、テンプレートか何かの段階で` が ' になっているかもしれません。

Copy link
Owner Author

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:

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 を設定したり、足したりしていませんか?
答えを間違えるケースもありますね。

Copy link
Owner Author

Choose a reason for hiding this comment

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

想定通り動いていないというのは, LeetCodeで出てきたテストケース以外だと動かないのではないかということで合っていますかね?

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に取れば良い話である
Copy link

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]
Copy link

Choose a reason for hiding this comment

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

at_idx = email.find("@") のほうが読みやすいと思います。

Copy link
Owner Author

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]
Copy link

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 = ""
Copy link

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 という変数名でもよいと思います。

Copy link
Owner Author

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という規格があるそうだ. 初めて知った
Copy link

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)
Copy link

Choose a reason for hiding this comment

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

+ が複数回出現する文字列の連結は、 f 文字列で書いたほうが読みやすい場合があります。
f"{local}@{domain}"
今回の場合は + で十分読みやすいと思います。

Copy link
Owner Author

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のドキュメント

  • (パッと見ですが)フォーマットに柔軟

Copy link

Choose a reason for hiding this comment

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

個人的な感覚となりますが、その認識で十分だと思います。 Python 3.6 で f 文字列が導入されてからは、そちらを優先的に使うように思います。

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.

4 participants