929. Unique Email Addresses - #14
Conversation
| def is_valid(email: str) -> bool: | ||
| if not 1 <= len(email) <= 100: | ||
| return False | ||
| parsed_email = re.fullmatch(r"[a-z0-9][a-z0-9.\+]*@[a-z0-9.\+]+\.com$", email) |
There was a problem hiding this comment.
全体的によく検討されていると思いました
コメントすることがあまりなかったので，local-partの末尾にはピリオドは許可されていないという豆知識だけコメントしておきます（現実のvalidationは難しいよね，程度の話です）
https://www.rfc-editor.org/info/rfc5321/#section-4.1.2:~:text=Local%2Dpart%20%20%20%20%20%3D%20Dot%2Dstring%20/%20Quoted%2Dstring%0A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%3B%20MAY%20be%20case%2Dsensitive%0A%0A%0A%20%20%20Dot%2Dstring%20%20%20%20%20%3D%20Atom%20*(%22.%22%20%20Atom)
There was a problem hiding this comment.
ありがとうございます。
local-partの末尾にはピリオドは許可されていないという豆知識だけコメントしておきます
こちら存じ上げなかったので勉強になりました！
（現実のvalidationは難しいよね，程度の話です）
確かにそうですよね、、
RFC と email サービスでの実際の実装のされ方が異なっていたりもしている (e.g., gmail では local-part の . を全て無視する) のもあって、何が正解か、どこまで許容するかを考えるのがより難しくなっている気がします。
| for email in emails: | ||
| canonicalized_emails.add(canonicalize(email)) | ||
| return len(canonicalized_emails) | ||
| ``` |
There was a problem hiding this comment.
自分は文字列判定でしかやっていなかったので、State Machineは関数内定義をしているやり方を含めて勉強になりました。
ただ、今の書き方だとステート（フラグ）と文字列判定がif分岐で混ざっているので、State Machineに振り切るなら以下のような書き方もよいと思います。
def canonicalize(email: str) -> str:
canonicalized = []
state = "LOCAL"
for c in email:
if state == "LOCAL":
if c == "@":
state = "DOMAIN"
canonicalized.append(c)
elif c == "+":
state = "ALIAS"
elif c != ".":
canonicalized.append(c)
elif state == "ALIAS":
if c == "@":
state = "DOMAIN"
canonicalized.append(c)
elif state == "DOMAIN":
canonicalized.append(c)
return "".join(canonicalized)There was a problem hiding this comment.
ありがとうございます。
今の書き方だとステート（フラグ）と文字列判定がif分岐で混ざっている
この視点はありませんでした。言われてみれば確かにそうですね。
state machine に振り切る形で解法を追加してみました。より状態遷移 (local -> alias -> domain)がはっきり見える形になって好みです。
58be1c7
There was a problem hiding this comment.
状態遷移を表す変数の型を enum にすると、読み手にとって意図が伝わりやすくなると思いました。
https://docs.python.org/ja/3/library/enum.html
There was a problem hiding this comment.
ありがとうございます。
一関数のために enum を定義して使うのは too much かなと思って使いませんでした。
https://github.com/kazizi55/coding-challenges/pull/14/changes#diff-237d647e27e9256f780ff8e7f2f9eb1acddf40099a005feec53f98fdb8416235R238-R239
が、おっしゃる通り、状態遷移の意図がより伝わりやすくなるのと、将来的なコードの拡張性などを考慮すると、enum を定義するのもありだなと思うようになりました。
| local, domain = tuple(email.split("@")) | ||
| stripped_local = local.strip(".") | ||
| stripped_local = stripped_local.split("+")[0] | ||
| return stripped_local + "@" + domain |
There was a problem hiding this comment.
こちらのコメントをご参照ください。
komdoroid/arai60#16 (comment)
There was a problem hiding this comment.
A single join with + is okay but do not format with +.
こちら存じ上げなかったので、勉強になりました。ありがとうございます。
https://google.github.io/styleguide/pyguide.html#310-strings
| for email in emails: | ||
| canonicalized_emails.add(canonicalize(email)) | ||
| return len(canonicalized_emails) | ||
| ``` |
There was a problem hiding this comment.
状態遷移を表す変数の型を enum にすると、読み手にとって意図が伝わりやすくなると思いました。
https://docs.python.org/ja/3/library/enum.html
4 participants
https://leetcode.com/problems/unique-email-addresses/description/
Next: https://leetcode.com/problems/first-unique-character-in-a-string/