Skip to content
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

First Unique Character in a String #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions pullrequests/first_unique_character_in_a_string/step1.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//lint:file-ignore U1000 Ignore all unused code
package template

/*
時間:5分
これまでに似たような問題を何個か解いたことがあるので比較的早く解くことができた。
1回のループで解く(notRepeatedのリストを保持して重複するごとに削除していく)ことも考えたが、1回のループで倍の処理をするのであれば2回ループして処理するのと全体の仕事量としては変わらないと思い、よりシンプルでわかりやすい方法にした。

-1をreturnするのは行儀が良くないが、LeetCodeの制約がなければ返り値としてerrを返すのが一番良いと思う。該当する要素が存在しないことは正常な挙動の範疇なのでlog.Fatalはするべきではないし、ログに記録するほどのものでもないと思う。

また、「サロゲートペアや合成文字等、char の値を複数用いないと表現出来ない文字が来ないことを想定」している。(https://github.com/seal-azarashi/leetcode/pull/15#discussion_r1704423277)
Copy link

Choose a reason for hiding this comment

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

文字コードは魔境であるという認識は大事でしょう。すべてを知っている人はいないと言っていいと思います。

あとは、LinkedHashMap 相当のものを実装してみるのは一つかもしれません。

*/
func firstUniqCharStep1(s string) int {
frequency := make(map[rune]int)
for _, r := range s {
frequency[r]++
}
for i, r := range s {
if frequency[r] == 1 {
return i
}
}
return -1
}
40 changes: 40 additions & 0 deletions pullrequests/first_unique_character_in_a_string/step2.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//lint:file-ignore U1000 Ignore all unused code
package template

/*
入力にアルファベットの小文字しかないので配列でも実装してみた。
mapを使った方法では実行時間が30~35msほどだったのに対し、配列を使う方法では10msほどである。
この理由として主にキャッシュ効率とオーバーヘッドの違いが考えられる。

配列は要素が連続したメモリ領域に配置されるのに対し、mapでは内部的には8つのエントリを持つバケットから構成されていて、各バケットはメモリ上で異なる位置に存在するため、参照局所性が悪い。
また配列では要素が26個しかないことからサイズは26*8B=208Bより、十分1次キャッシュに載るのに対し、mapは1エントリあたり4B+8B=12Bで、最大サイズは12B*10^5=1.2GBになることから3次キャッシュにも載り切らないというのもある。

また、mapはハッシュ化のオーバーヘッドがあるのと、Step1の場合はcapacityを指定せずにmapを作成したため、Load factorがある閾値(6.5)を超えるとリサイズが行われ、リハッシュする必要が生じる。
よって主に上記の2つを要因としてmapの方が実行時間が遅くなったと考えられる。
ちなみにStep2ではcapacityを指定してmapを作成したところ、実行時間は20~25msとなった。
*/
func firstUniqCharStep2(s string) int {
frequency := make(map[rune]int, len(s))

Choose a reason for hiding this comment

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

sにユニークな文字が少ないとき、余分にメモリを確保することになるのでトレードオフの議論は必要そうかなと思いました。

Copy link

Choose a reason for hiding this comment

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

キャパシティはlen(s)ではなく、26ではないでしょうか?

for _, r := range s {
frequency[r]++
}
for i, r := range s {
if frequency[r] == 1 {
return i
}
}
return -1
}

func firstUniqChar2Step2(s string) int {
var frequency [26]int
for _, r := range s {
frequency[r-'a']++
}
for i, r := range s {
if frequency[r-'a'] == 1 {
return i
}
}
return -1
}