Idiomatic way to refer to reference or value

help
1

Hello! I'm still very new and trying to learn the right way to do things without just cloning my way out of a problem :smiley:

Suppose I have some structs struct:

struct Data {
    pub x: i32,
    pub y: i32, 
}

struct Agent {
   q: i32,
   r: i32,
}

impl Agent {
    pub fn get_data(&self) -> Data {
        Data { x: self.q, y: self.r }
    }

Now, suppose I have a method where someone can optionally send alternative Data:

fn do_stuff(agent: &Agent, data: Option<&Data>) {
    let our_data: &Data;
    if data.is_none() {
        our_data = &agent.get_data();
    } else {
        our_data = data.unwrap();
    }

    do_other_things(our_data);
    [...]
}

This doesn't work because &robot.get_data() creates a temporary value that goes out of scope. I know that one way I can fix this is to have the do_stuff function just taken an Option<Data> but then it seems like the called would need to call it with cloned Data since it can't just send a reference.

So how would people solve this problem?

2

One simple way is to just move do_other_things inside the branches:

fn do_stuff(agent: &Agent, data: Option<&Data>) {
    match data {
        None => do_other_things(&agent.get_data()),
        Some(data) => do_other_things(data),
    }
}
3

It is more idiomatic to use match as an expression here instead of forward declaring our_data.

#[derive(Clone)]
struct Data {
    pub x: i32,
    pub y: i32,
}

struct Agent {
    q: i32,
    r: i32,
}

impl Agent {
    pub fn get_data(&self) -> Data {
        Data {
            x: self.q,
            y: self.r,
        }
    }

    fn do_stuff(agent: &Agent, data: Option<&Data>) {
        let our_data = match data {
            // Note: We need to clone the inner `&Data` because 
            // `Agent::get_data()` returns an owned `Data` and not a borrowed 
            // one (like the `data` parameter)
            Some(d) => d.clone(),
            None => agent.get_data(),
        };

        todo!()
    }
}

(playground)

Your original code won't compile as-is because Agent::get_data() constructs a new (owned) Data, whereas the thing inside the data parameter is borrowed. I needed to make a copy of the data parameter's contents (d.clone()) so both branches have the same type.

4

If you don't want to repeat the function call, you can extend the lifetime of the temporary by lifting it out of the if expression:

fn do_stuff(agent: &Agent, data: Option<&Data>) {
    let agent_data;
    let our_data = match data {
        None => {
            agent_data = agent.get_data();
            &agent_data
        }
        Some(data) => data,
    };
    do_other_things(our_data)
}

Rust's branch analysis is powerful enough in this case to figure it out.

5

Additionally, if Data and Agent implement Clone (we won't actually clone it), you can use Cow to clarify the semantics:

fn do_stuff(agent: &Agent, data: Option<&Data>) {
    use std::borrow::Cow;

    let our_data = match data {
        None => Cow::Owned(agent.get_data()),
        Some(data) => Cow::Borrowed(data),
    };
    do_other_things(&our_data)
}

From the definition you showed, Data and Agent should have no trouble implementing Clone, I suppose.

6

In the example, clone isn't bad. But in reality, it's a pretty big struct with lots of data in it, which is why I was hoping to avoid cloning.

7

A quick question about this solution: under the hood, is the compiler still cloning the data or is it smartly just passing ownership of the memory? If this struct was something like 8 gigs of memory, am I going to have issues with perf?

8

Assuming that the 8GiB of memory is stored on the heap (since it's probably gonna overflow the stack), and the structs merely hold a pointer to it, no, there is no cloning going on.