Review on this tree implementation

help
1

Hi!

I’m a new rustacean and as a learning project I’m developing a little game engine.
I’ve just made a little piece, and I would like to have your review.
To put some context, the following code is the implementation of a « Transform », it is used to set the position and hierarchy of my Entities (or game objects if you are familiar with the Unity3D world).
It is a simple tree. Each node can have one parent and can have many children. A node has a set_parent method which set the parent to his context, but also set itself as a child in the parent.

I would like to know if my implementation follows the rust philosophy.
I heard that RefCell can rime with code smell, do you think it is the case here? If yes, do you have some suggestion to improve this?
Do you have some suggestions in general?
Do you see different approaches to this problem which could be better?

Also I would like the set_method function to be a member function of Transform instead of being a static function, how to do that?

Thanks!

use std::rc::Rc;
use std::rc::Weak;
use std::cell::RefCell;

#[derive(Debug)]
pub struct Transform {
    id: i64,
    parent: Option<Rc<RefCell<Transform>>>,
    children: Rc<RefCell<Vec<Weak<RefCell<Transform>>>>>,
}

impl Transform {
    pub fn new(id: i64) -> Rc<RefCell<Transform>> {
        Rc::new(RefCell::new(Transform {
            id,
            parent: None,
            children: Rc::new(RefCell::new(vec![])),
        }))
    }

    pub fn set_parent(self_: Rc<RefCell<Transform>>, parent: Rc<RefCell<Transform>>) {
        self_.borrow_mut().parent = Some(parent.clone());

        let borrowed_parent = parent.borrow();
        borrowed_parent.children.borrow_mut().push(Rc::downgrade(&self_));
    }

    pub fn get_children(&self) -> Rc<RefCell<Vec<Weak<RefCell<Transform>>>>> {
        self.children.clone()
    }

    pub fn get_id(&self) -> i64 {
        self.id
    }
}

fn main() {
    let root = Transform::new(0);
    let manager1 = Transform::new(1);
    let manager2 = Transform::new(2);

    Transform::set_parent(manager1.clone(), root.clone());
    Transform::set_parent(manager2.clone(), root.clone());

    println!("{:#?}", root);
    println!("{:#?}", manager1);
    println!("{:#?}", manager2);

    // Iterate over children and print it
    {
        let borrowed_root = root.borrow();
        let root_children = root.borrow().get_children();
        let borrowed_children = root_children.borrow();

        for transform_child in borrowed_children.iter() {
            match transform_child.upgrade() {
                None => println!("None"),
                Some(transform) => {
                    let borrowed_transform = transform.borrow();

                    println!(
                        "Tranform {} has child {}",
                        borrowed_root.get_id(),
                        borrowed_transform.get_id()
                    );
                }
            }
        }
    }
}
2

Set_parent. I feel your pain. Struggled for ages before I concluded that it had to be a static method otherwise how could you get a handle on the Rc that you are wrapped by. I would love to know if there’s an elegant way around that.

My tree had Rc<RefCell> and a weak parent but looked pretty similar. Can children just be Vec<Weak<RefCell>? Two sets of RefCells feels wrong but I am learning also.

1 Like
3

Oh yes, you are right @gilescope! Thank you :slight_smile:

Here the new version:

use std::rc::Rc;
use std::rc::Weak;
use std::cell::RefCell;

#[derive(Debug)]
pub struct Transform {
    id: i64,
    parent: Option<Rc<RefCell<Transform>>>,
    children: Vec<Weak<RefCell<Transform>>>,
}

impl Transform {
    pub fn new(id: i64) -> Rc<RefCell<Transform>> {
        Rc::new(RefCell::new(Transform {
            id,
            parent: None,
            children: vec![],
        }))
    }

    pub fn set_parent(self_: Rc<RefCell<Transform>>, parent: Rc<RefCell<Transform>>) {
        self_.borrow_mut().parent = Some(parent.clone());

        let mut borrowed_parent = parent.borrow_mut();
        borrowed_parent
            .children
            .push(Rc::downgrade(&self_));
    }

    pub fn get_children(&self) -> Vec<Weak<RefCell<Transform>>> {
        self.children.clone()
    }

    pub fn get_id(&self) -> i64 {
        self.id
    }
}

fn main() {
    let root = Transform::new(0);
    let manager1 = Transform::new(1);
    let manager2 = Transform::new(2);

    Transform::set_parent(manager1.clone(), root.clone());
    Transform::set_parent(manager2.clone(), root.clone());

    println!("{:#?}", root);
    println!("{:#?}", manager1);
    println!("{:#?}", manager2);

    // Iterate over children and print it
    {
        let borrowed_root = root.borrow();
        let root_children = root.borrow().get_children();

        for transform_child in root_children.iter() {
            match transform_child.upgrade() {
                None => println!("None"),
                Some(transform) => {
                    let borrowed_transform = transform.borrow();

                    println!(
                        "Tranform {} has child {}",
                        borrowed_root.get_id(),
                        borrowed_transform.get_id()
                    );
                }
            }
        }
    }
}