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

add dsu document(ja) #13

Merged
merged 3 commits into from
Sep 21, 2020
Merged

add dsu document(ja) #13

merged 3 commits into from
Sep 21, 2020

Conversation

universato
Copy link
Owner

@universato universato commented Sep 14, 2020

DSUのドキュメントの日本語版を追加します。

本家ACLは、本体コードのライセンスは決まっていますが、ドキュメントの方は現在まだです。
ドキュメントのライセンスも本体コードと同じだろうと推定しています。

しかし、もし本家のドキュメントのライセンスが想定よりも厳しければ、本家ACLを見ながら書いたせいで全く同じ文言になっているところもあるので変更する必要があるかもしれないです。

書いてて思ったのですが、本家ACLmerge(a, b)と書くと頂点aと頂点bぽく見えすが、leader(a)と書くと配列ぽく見えるので、頂点の変数がvやwにしたい気持ちがあります。本家ドキュメントに寄せたい気持ちもあるので、いったん本家に合わせました。


`DSU`, `DisjointSetUnion`, `UnionFind`, `UnionFindTree`

## merge(a, b)
Copy link
Contributor

Choose a reason for hiding this comment

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

実装にあわせるため、戻り値を Integer にして「a, bが連結だった場合はその代表元、非連結だった場合は新たな代表元を返します」といった説明を追加するのはどうでしょうか。(本家より引用した文です。)

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でmergeだけ戻り値を書いてないのは忘れているわけではなく、色々思うことがあり、実はあえて書かなかったです。
というのも、「元から連結だから新たに連結しなかったときはfalseで、非連結から連結できたときはtrueを返す」仕様にした方が使い勝手が良いと考えており、実際そういう問題は思い浮かぶのですが(ABC120 D - Decayed Bridges)、反対にmerge実行後に代表元が欲しい問題が思い浮かばなかったです。

  1. 本家ACLに真偽値を返すように要請してもし真偽値を返す実装に変更してもらったら、こちらも実装を変更したい。
    1. 本家の実装から離れて1-based indexにして、新たに連結したときは正の値で頂点番号を返して、そうでないときは負の値で頂点番号を返して情報量を増やす。
    2. 0-based indexのまま、新たに連結したときは非負整数で頂点番号を返し、そうでないときは負の数で頂点番号を返す。こちらは線対称にならないのが、微妙ですね。
  2. 本家にはないmerge?メソッドを加えて、merge?は真偽値を返す実装にする。

まだこれ以外にも選択肢がありそうですが、3の実装がちょっとRubyぽくて良いかなと思いました。2は1つのメソッドで情報量が増えて一挙両得な感じもしますが、一般的じゃなくオシャレな感じがしないので否定的です。

Copy link
Contributor

Choose a reason for hiding this comment

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

ありがとうございます。
一旦ドキュメントとしては何も書かないまたは本家に合わせる、のどちらかに倒しておいて、issueで DSU#merge の挙動を議論するのもありかなと思いました。

Copy link
Contributor

Choose a reason for hiding this comment

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

3でmerge?メソッドを加えて、返す値は新しく連結したとき代表元、そうでないときfalseにしたら情報量もそこそこあって面白いと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

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

ドキュメントは本家に合わせる方向にして、kotatsugameさんの方針でmerge?メソッドを付け加えるPRだそうと思います! ありがとうございます!

【追記】
ちょっと書き忘れたのですが、nonzero?nilselfを返すので、falseと代表元(integer)を返すのRubyらしい感じがします。

@universato
Copy link
Owner Author

勝手に「使っている人もいたよな」と思いunionメソッドをエイリアスとして追加してましたが、
uniteメソッドというエイリアスがあれば十分で、unionという名詞は使って欲しくないと思ったので、削除しました。
なお、本家ACLがmergeで、蟻本はunite、螺旋本はlinkでした。螺旋本のlinkというメソッドもいいなと思いましたが、いったん見送りです。
そろそろいったんマージします。

@universato universato merged commit dc796a6 into master Sep 21, 2020
@universato universato mentioned this pull request Sep 21, 2020
@universato universato deleted the add-dsu-doc branch September 21, 2020 20:02
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.

None yet

3 participants