Alternative to unwrap when moving a field of a struct

This was difficult to title, so please bare with me while I try to explain :slight_smile:

I am converting from one struct BigThing with lots of optional fields to a smaller, more specialised struct SmallThing with fewer fields that aren't optional.

I ended up with the following code, but I'm curious whether there are alternative ways to write this that avoid infallible unwraps - or at least, what I think are infallible unwraps!

impl TryFrom<BigThing> for SmallThing {
    type Error = Report<ConvertError<SmallThing>>;

    fn try_from(big: BigThing) -> Result<Self, Self::Error> {
        // check if it's none, avoiding moving the sub field
        if big.data.code.is_none() {
            return Err(ConvertError::new())
                .into_report()
                // this moves the BigThing, which causes my headache
                .attach_lazy(|| big)?;
        }

        if big.data.text.is_none() {
            return Err(ConvertError::new())
                .into_report()
                // as does this
                .attach_lazy(|| big)?;
        }

        let code = big.data.code.unwrap();
        let text = big.data.text.unwrap();

        Ok(SmallThing {
            metadata: big.metadata,
            code,
            text,
        })
    }
}

Trying the "obvious?" thing, I get errors due to a partially moved value, and I can see what the compiler is saying.

let code = big.data.code
    .ok_or_else(ConvertError::new)
    .into_report()
    .attach_lazy(|| big)?;
//               ^^^^^^ error[E0382]: use of partially moved value

ok_or_else moves self, so I lose the big.data.code field, and therefore can't use big at all later.

pub fn ok_or_else<E, F>(self, err: F) -> Result<T, E>

I could clone big, but since I only want to move when there's an error I'd rather avoid the cost to do so.

Should I be satisfied with my unwrap strategy?
Can you think of a nicer solution?

Oh, and if you're curious, Report, into_report() and attach_lazy come from error_stack which I've been enjoying using so far! It can be verbose but it feels like it might be the right error handling solution for me.

It's not supported in the playground but I rigged something up to demonstrate the problem: Rust Playground

If I understand the crate correctly [1], I believe you can do something like:

    fn try_from(mut big: BigThing) -> Result<Self, Self::Error> {
        let code = match big.data.code.take() {
            Some(code) => code,
            e => return ConvertError::new()
                .report()
                .attach_lazy(|| big),
        };
        // similar for text

        Ok(SmallThing { metadata: big.metadata, code, text, })
    }

And a helper function can shorten that to:

    fn try_from(mut big: BigThing) -> Result<Self, Self::Error> {
        let code = match big.data.code.take() {
            Some(code) => code,
            e => return ConvertError::with(big),
        };
        // similar for text

        Ok(SmallThing { metadata: big.metadata, code, text, })
    }

And when/if let-else stabilizes, you could then do:

    fn try_from(mut big: BigThing) -> Result<Self, Self::Error> {
        let Some(code) = big.data.code.take() else { return ConvertError::with(big) };
        let Some(text) = big.data.text.take() else { return ConvertError::with(big) };
        Ok(SmallThing { metadata: big.metadata, code, text, })
    }

  1. boy I find their aliasing over the standard Result annoying ↩ī¸Ž

4 Likes

If it's important to have code and text on the error report (take will remove code before text is checked) you can also match on them instead

fn try_from(mut big: BigThing) -> Result<Self, Self::Error> {
    Ok(match (big.data.code, big.data.text) {
        (Some(code), Some(text)) => SmallThing {
            metadata: big.metadata,
            code,
            text,
        },
        (code, text) => {
            // Replace whatever the previous value was on `big` so we can move it again.
            big.data.code = code;
            big.data.text = text;
            return Err(ConvertError::new())
                .into_report()
                // this moves the BigThing, which causes my headache
                .attach_lazy(|| big);
        }
    })
}
3 Likes

Or just... match it all.

    fn try_from(big: BigThing) -> Result<Self, Self::Error> {
        match big {
            BigThing {
                metadata,
                data: Data {
                    code: Some(code),
                    text: Some(text),
                    ..
                }
            } => Ok(SmallThing { metadata, code, text }),
            _ => Err(ConvertError::new()).into_report().attach(big)
        }
    }
7 Likes

Thank you both so much. take() is a really interesting tool, and I'd somehow neglected to consider using match!

It is important for me to return the original activity as it was, and using take gave me an idea to also use replace:

let code = match big.data.code.take() {
            Some(code) => code,
            e => return ConvertError::new()
                .report()
                .attach_lazy(|| big),
        };

let text = match big.data.text.take() {
            Some(code) => code,
            e => {
                // replace each field we took out previously
                big.data.code.replace(code);
                return ConvertError::new()
                    .report()
                    .attach_lazy(|| big),
            }
        };

I'll have to experiment to see what scales well when there are more and more fields (and other validation logic to boot!).

I am hopeful the match it all strategy will work well since it won't give me the option to forget to replace anything!

Update: just matching it all is actually really nice now I'm less shocked by the size of it :smiley: