solve: 141.Linked List Cycle

[t9a-dev]

問題: 141. Linked List Cycle
次に解く問題: 142. Linked List Cycle II
ファイルの構成: ./src/bin/<各ステップ>.rs

[oda]

うーん、こういうヘルパーを用意すると少しきれいになりますかね。

fn next_node(node: Option<Rc<RefCell<ListNode>>>) -> Option<Rc<RefCell<ListNode>>> {
    node.and_then(|n| n.borrow().next.clone())
}

[t9a-dev]

ありがとうございます。メソッドチェーンが長いなとは書いていて思ったのですが、言語仕様的に必要な呼び出しだから仕方ないかと違和感をスルーしていた部分でした。
ご指摘いただいてから改めて考えみると、ロジックに直接関係ない冗長な部分をヘルパーに切り出すのは納得できる理由だと思いました。
だいぶすっきり書けるなと思いました。

pub fn has_cycle(head: Option<Rc<RefCell<ListNode>>>) -> bool {
    let (mut slow_node, mut fast_node) = {
        let current = head.as_ref().map(Rc::clone);
        let next = Self::next_node(head);

        (current, next)
    };

    while let (Some(slow), Some(fast)) = (slow_node, fast_node) {
        if Rc::ptr_eq(&slow, &fast) {
            return true;
        }

        slow_node = Self::next_node(Some(slow));
        fast_node = Self::next_node(Self::next_node(Some(fast)));
    }

    false
}

fn next_node(node: Option<Rc<RefCell<ListNode>>>) -> Option<Rc<RefCell<ListNode>>> {
    node.and_then(|n| n.borrow().next.as_ref().map(Rc::clone))
}

[TrsmYsk]

変数名ですがnodeのままでもいいかなと思います。
current_nodeだとcurrentと対になるようなノード(previous, nextなど)があるかもしれないという連想が働くので、ちょっとだけ読み手のコストが増す気がします。
Zun-U/coding-practice-mochi0123#2 (comment)

[naoto-iwase]

お疲れさまです。

LeetCodeにRust実装がなかったとのことで、大変だったと思います。丁寧に実装を考えられているという印象を持ちました。

1点だけコメントさせていただきました。

[naoto-iwase]

Option<Rc<RefCell<ListNode>>>がボイラープレートっぽいので、型エイリアスを使うのはどうでしょうか。

Suggested change

	pub struct ListNode {
	pub next: Option<Rc<RefCell<ListNode>>>,
	}
	type Link = Option<Rc<RefCell<ListNode>>>;
	pub struct ListNode {
	next: Link,
	}

テストとか書くときも少し気が楽になるかもです。

[t9a-dev]

ありがとうございます。型エイリアスは活用したことがなく提案いただくまで選択肢にもなかったので、すごく勉強になりました。（アハ体験でした。）

[Zun-U]

レビューが遅くなり、申し訳ありません。
next_nodeの命名がなるほど、と思った反面、「next」という単語の意味に「現時点から次」と連想してしまう場合もあると思いますので、target_nodeにする案はどうでしょうか。