Implementing Display trait on a type that returns a mutable reference

I have a type (A) that contains another type (B). I have (A) return a mutable reference to (B) so we can update it nicely:

struct A {
	b: B,
}

struct B {
	value: u32,
}

impl A {
	fn new() -> Self {
		Self {
			b: B::new(),
		}
	}
	
	fn b(&mut self) -> &mut B {
		&mut self.b
	}
}

impl B {
	fn new() -> Self {
		Self {
			value: 5,
		}
	}
	
	fn value(&self) -> u32 {
		self.value
	}
	
	fn update(&mut self, new_value: u32) {
		self.value = new_value;
	}
}

fn main() {
	let mut a = A::new();
	
	println!("value is: {}", a.b().value());
	a.b().update(10);
	println!("value is: {}", a.b().value());
}

This works just fine:

value is: 5
value is: 10

But now I want to implement the Display trait for (A):

impl fmt::Display for A {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "{}", self.b().value())
    }
}

When adding this code, I get the following error:

error[E0596]: cannot borrow `*self` as mutable, as it is behind a `&` reference
  --> src\main.rs:49:25
   |
49 |         write!(f, "{}", self.b().value())
   | 

I understand what the error is telling me. But I'm not sure the best idiomatic way to fix it.

I got it to work by making separate functions in impl A, where B can be returned as either mutable or immutable:

impl A {
	fn new() -> Self {
		Self {
			b: B::new(),
		}
	}
	
	fn b(&self) -> &B {
		&self.b
	}

	fn b_mut(&mut self) -> &mut B {
		&mut self.b
	}
}

But this seems a little sloppy to me. I figure there are three possibilities:
(1) My initial approach is not idiomatic, and there's a more appropriate way to accomplish this
(2) There's actually an easy fix with just a few keystrokes, that I haven't been able to find in my searches
(3) This is just the way it is

Any advice on this situation would be most helpful.

it's your type, you are the author, why not just use the field directly, instead of calling the method that returns a reference?

-        write!(f, "{}", self.b().value())
+        write!(f, "{}", self.b.value())

feel free to provide more context about the actual problem if your example code didn't resemble the real constraints.

this is quite common practice in rust actually, what makes you think it's "sloppy"? again more context is needed to better understand your problem.

3 Likes

Oh geez, I feel dumb. I've had it drilled into my brain from older languages that making "member variables" public was bad form, so I guess my mind didn't even consider it for this application.

That did the trick, thank you!

Question: Is this approach typically acceptable in libraries?

I actually learned about that approach here. By "sloppy" I meant for this particular problem specifically, since it required an addition that didn't seem necessary.

do you mean accessing the field directly? I don't understand why you are reluctant to access a struct field directly. when implementing a trait for a type, either the type is defined in your own crate, or the trait is defined in your own crate (or both), this is referred as the orphan rule. in this case, the trait Iterator is a foreign trait (from core) so you can only implement it for your own types, thus it counts as "implementation details".

a "getter" method is unnecessary in the first place, if all it does is to hide the field access. use "getter" methods only if your type must maintain certain invariant and exposing the field directly can lead to invalid states for your type. otherwise, just let the user access the field directly.

on the other hand, it's very common to see pairs of methods that differ only in mutability, both in the standard library and in third party libraries. for instance: for core::Option, there are as_ref() vs as_mut(), as_deref() vs as_deref_mut(), for core::cell::RefCell, we have borrow() vs borrow_mut(), for any conainer C, we have C::iter() vs C::iter_mut(). in third party libraries like indexmap, either, you'll see plenty of these pairs.

note these are not "getters", they are mostly type conversions or projections, or conveys special semantics.

3 Likes

You don't need to make the field public for this. 1st-party impls of a type always have access to its fields. Just think about it: if this weren't the case, all non-public access levels would be useles. If even the impl of the type itself can't access its fields, then who could?

If not adding that method would lead to a type error, which is solvable by adding an other method, then what's unnecessary about it?

1 Like