Skip to content

929. Unique Email Addresses#14

Open
hroc135 wants to merge 4 commits intomainfrom
929UniqueEmailAddresses
Open

929. Unique Email Addresses#14
hroc135 wants to merge 4 commits intomainfrom
929UniqueEmailAddresses

Conversation

@hroc135
Copy link
Copy Markdown
Owner

@hroc135 hroc135 commented Aug 16, 2024

continue
}

simplifiedEmail += string(c)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

文字列の結合はコストが高く、この場合はbytes.Bufferを使った方が速くなるようです。

	var simplifiedEmail bytes.Buffer
	// 中略
		simplifiedEmail.WriteRune(c)
	// 後略

Leetcodeで何度か試した感じだと、1/2〜1/3ぐらいになっていそう?たまに0msとかも出るぐらいになりました。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

勉強になります!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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


func simplifyEmailAddress(email string) string {
simplifiedEmail := ""
localOrDomain := "local"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

localOrDomainは2値で、他にモードが増える事が考えにくい気がしたのでtrue/falseで持ちたい気もしますが、好みによるかもしれません。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

同じことを思いました。定数として表現する場合、Goには言語機能としてenumはないようですが、表現する方法はいくつかあるようなので、そちらを採用するとよりバグフリーなコードになると思います。
https://stackoverflow.com/questions/14426366/what-is-an-idiomatic-way-of-representing-enums-in-go

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ご指摘ありがとうございます。true/falseの場合、例えばisLocal = falseの時にドメインを探索しているということがわかりにくいと思いました。
enumっぽいiotaは使っていこうと思いました

```

- 標準ライブラリのstringsを使って解く
- 時間計算量はstep1の解法と同じO(n)のはずなのにこちらの方がleetcodeの計測時間が1/3くらいになるのは標準ライブラリの最適化のおかげ?
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

素朴な文字列結合の繰り返しが高コストと思われます(Step1のコメント参照)


for _, email := range emails {
localAndDomain := strings.Split(email, "@")
if len(localAndDomain) != 2 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ありがとうございます!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

こういうのを自分で引っ張って来れるようなマインドセットが大事なんですよね


func simplifyEmailAddress(email string) string {
simplifiedEmail := ""
localOrDomain := "local"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

同じことを思いました。定数として表現する場合、Goには言語機能としてenumはないようですが、表現する方法はいくつかあるようなので、そちらを採用するとよりバグフリーなコードになると思います。
https://stackoverflow.com/questions/14426366/what-is-an-idiomatic-way-of-representing-enums-in-go

return distinctSimplifiedEmails
}

func simplifyEmailAddress(email string) string {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

簡素化するというよりは正規化するという方がしっくりきました。normalizeEmailAddressなどはいかがでしょうか。

}

local, domain := localAndDomain[0], localAndDomain[1]
if len(local) == 0 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

実務ではemailのvalidationはしっかりテストされたライブラリを見つけるというのも一つの手かと思いました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

実際の業務でユーザーが入力した値を扱うとなると、どんな値が入ってくるか分からないので(問題には constraints があるとはいえ)別途バリデーションが必要なことは示せていた方が、面接官からは評価されるだろうなと思いました。
面接中に口頭で言ってもいいですし、コードコメントにや Go Doc に書いてしまってもいいかも知れません。

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

そうですね。面接の時はおそらく最初から制約は与えてくれないと思うので、自分で聞きながら確認することになるだろうと思います

localName = strings.Split(localName, "+")[0]
localName = strings.ReplaceAll(localName, ".", "")

normalized := localName + "@" + domainName
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Eメールの正規化部分は関数化されていると読みやすいです。
これは好みですが、自分は正規化処理自体も個々の操作の関数の組み合わせで書かれているとわかりやすいと思います。

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.

6 participants