-
Notifications
You must be signed in to change notification settings - Fork 0
929. Unique Email Addresses #15
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
| unique_emails = set() | ||
| for email in emails: | ||
| local_name, domain_name = email.split('@') | ||
| formatted_local_name = local_name.replace('.', '').split('+')[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.
1 行に多くの要素を詰め込み過ぎているように感じました。
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.
ありがとうございます。2行に分けた方が良さそうですね。
|
|
||
| return len(unique_emails) | ||
|
|
||
| def clean_local_name(self, 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.
private メソッド相当ですので、関数名を _ で始めると良いかもしれません。
また、local_name をきれいにするというよりは、正規化するといったほうがしっくりきます。 _normalize_local_name() はいかがでしょうか? _canonicalize_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.
たしかにクレンジングより正規化の方がしっくり来ました。_normalize_local_name() いいですね、ありがとうございます。
underscoreの話はこの辺りですかね。
Use one leading underscore only for non-public methods and instance variables.
https://peps.python.org/pep-0008/#method-names-and-instance-variables
| def remove_all_dots(self, text): | ||
| return text.replace('.', '') | ||
|
|
||
| def remove_after_plus(self, text): |
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.
細かい点となりますが、 after plus ですと、 plus より後を表すように思います。 remove_plus_and_after() はいかがでしょうか?
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.
ありがとうございます。remove_plus_and_after() や pick_before_plus() が良さそうです。
| unique_emails = set() | ||
| for email in emails: | ||
| local_name, domain_name = email.split('@') | ||
| local_name = re.sub(r'\+.*', '', 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.
個人的な感覚ですが、組み込み関数と正規表現で、同じ処理を粉う場合、組み込み関数を使ったほうが、読んでいて認知負荷が低く読みやすく感じます。
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.
参考になります。こちら自分も同じ感覚です。(正規表現にあまり慣れていないこともありますが...)
| def numUniqueEmails(self, emails: List[str]) -> int: | ||
| unique_emails = set() | ||
| for email in emails: | ||
| local_name, domain_name = email.split('@') |
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.
split, replace あたりの標準マニュアルは一回見ておきましょう。
https://docs.python.org/3.12/library/stdtypes.html#str.split
意外な引数や機能があったりするからです。
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.
いつもありがとうございます。
一通り目を通しておきます。
| def _remove_all_dots(self, text): | ||
| return text.replace('.', '') | ||
|
|
||
| def _remove_plus_and_after(self, text): | ||
| return text.split('+')[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.
1行のみの処理なら関数化しなくてもいいのでは、と思いました。複数箇所で使われていたり、よっぽど分かりづらい内容なら関数名がつくことで理解が捗ることもあるかも知れませんが、どちらの関数の処理も非常に明快に感じます。
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.
_normalize_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.
たしかにここは好みの問題かと思います。ありがとうございます。
問題へのリンク
https://leetcode.com/problems/unique-email-addresses/description/
次に解く問題
387. First Unique Character in a String
README.mdへ頭の中の言語化と記録をしています。