Pattern matching over entries of an array/split

42 Views Asked by At

I'm implementing a type of BSP tree in Rust, and I am wanting to write a function that tells which side of a parent node a node belongs to. This is what I have

enum Side {
    L = 0,
    R = 1,
}

enum RegionKind {
    Split { 
        subregion: [Option<Rc<Region>>; 2],
    },
    Client{ 
        window: i32,
    },
}

struct Region {
    kind: RegionKind,
    container: Option<Rc<Region>>,
    tags: u8,
}

impl Region {
    fn from(&self) -> Option<Side> {
        if let RegionKind::Split{subregion,..} = &self.container.as_ref()?.kind {
            match &subregion[0] {
                None => None,
                Some(r) => if eq(Rc::<Region>::as_ptr(&r),self) {
                    Some(Side::L)
                } else {
                    None
                }.or_else(|| { match &subregion[1] {
                    None => None,
                    Some(r) => if eq(Rc::<Region>::as_ptr(&r),self) {
                        Some(Side::R)
                    } else {
                        None
                    }
                }})
            }
        } else {
            None
        }
    }
}

There are currently two things in place that are making this function quite the behemoth.

  1. I need to repeat the check on on subregion[0] and subregion[1]. I would really like to be able to return the Side of the index (given that L = 0 and R = 1 like shown in the enum declaration) and be able to combine these into one, hence why I am asking to match over the elements of the array. Something like how you use Some(r) => ... to unbox the r out of the Some

  2. I need to check if the RegionKind is Split. I can assert that any Region's container will always have RegionKind::Split as their kind, so I would like to be able to eliminate the if let else entirely (I would prefer to panic!() in that else block if I must have it as well, but the compiler gets upset about return types so I need to add the None at the end anyways, which just ends up getting marked as unreachable)

I figure there's probably some arcane Rust syntax that can make this much more concise and I just don't know it or don't know how to employ it here. This same function can be a single line macro in C.

2

There are 2 best solutions below

1
drewtato On BEST ANSWER

I would move some stuff to other functions.

impl RegionKind {
    // you may want to make this a method on `Region` instead
    fn subregion(&self) -> Option<&[Option<Rc<Region>>; 2]> {
        if let RegionKind::Split { subregion, .. } = self {
            Some(subregion)
        } else {
            None
        }
    }
}

impl Region {
    fn eq_then_side(&self, r: &Region, side: Side) -> Option<Side> {
        eq(self, r).then_some(side)
    }
}

Then you can write it concisely.

pub fn from(&self) -> Option<Side> {
    let [sub_left, sub_right] = self.container.as_ref()?.kind.subregion().unwrap();

    sub_left
        .as_ref()?
        .eq_then_side(self, Side::L)
        .or_else(|| sub_right.as_ref()?.eq_then_side(self, Side::R))
}

Note that if you ever find yourself matching an Option, especially with None => None, you can almost certainly use an Option method instead, usually map or and_then. In this case, ? will work since the two None values are being returned.

I believe you don't need to use Rc::as_ptr since you are never using the pointer for reading, and the Rc is never dropped anyway. You can just derive the pointer straight from the shared reference.

What you said about panics isn't true. You can always replace an expression with a panic unless that return is needed for type inference, but since you have an explicit return type, that does not apply here. I've used unwrap above as the panic.

If you wanted to make it truly one expression, you can do that, but I wouldn't recommend it since it becomes too complex.

pub fn from(&self) -> Option<Side> {
    self.container
        .as_ref()?
        .kind
        .subregion()
        .unwrap()
        .iter()
        .map_while(|sub| sub.as_ref())
        .zip([Side::L, Side::R])
        .find_map(|(sub, side)| sub.eq_then_side(self, side))
}

You could also replace [Side::L, Side::R] with (0u8..).map(|n| Side::try_from(n).unwrap()) and implement TryFrom<u8> for Side, but I wouldn't bother when you only have two variants. Rust doesn't provide many convenient uses for explicit enum discriminants.

0
Chayim Friedman On

Since we know that if we have a parent we are either the left child or the right, we can avoid the check on the right (like in C). This simplifies the code:

fn from(&self) -> Option<Side> {
    let RegionKind::Split {
        subregion: [sub_left, _],
        ..
    } = &self.container.as_ref()?.kind
    else {
        unreachable!();
    };
    if let Some(sub_left) = sub_left {
        if eq(&**sub_left, self) {
            return Some(Side::L);
        }
    }
    Some(Side::R)
}

This is still longer than C for two reasons: first, in Rust you use reference counting and the layout of Rc is not guaranteed, so you cannot just compare self to Option<Rc> assuming None will return false. Second, you need to handle the case of RegionKind::Client, which you don't handle in C. The handling of a root node (no parent), however, is given almost for free, thanks to the ? operator.