How to make this vector of reference wrappers more ergonomic?

code review
1

I have an Extension struct that wraps a shared library and some other associated state. For convenience, I've created an ExtensionRef struct that wraps an extension along with its name (for ease of logging). It looks like this:

pub struct ExtensionRef
{
	name: String,
	extension: RefCell<Extension>,
}

impl ExtensionRef
{
	// Among other functions:

	pub fn get_extension(&self) -> Result<Ref<'_, Extension>, BorrowError>
	{
		return self.extension.try_borrow();
	}

	pub fn get_extension_mut(&self) -> Result<RefMut<'_, Extension>, BorrowMutError>
	{
		return self.extension.try_borrow_mut();
	}
}

Finally, I have an ExtensionList class which holds all the extensions I've loaded. It's really just a convenience for scanning a directory, loading shared libraries and checking that they expose a compatible interface; those that do are stored in a Vec<ExtensionRef>.

pub struct ExtensionList
{
	extensions: Vec<ExtensionRef>,
}

I find myself having to do a lot of for ext_ref in list.iter() and apply the same operation to each extension in turn. This means that I have to call ext_ref.get_extension().unwrap() (or ext_ref.get_extension_mut().unwrap()) a lot. Given I can control how this list is being used, and the stored ExtensionRefs are private to the list, it'd be nice to be able to essentially "lock" the list in a mutable or non-mutable way, and work with a set of borrowed &mut Extension or &Extension references respectively.

I have tried turning the Vec<ExtensionRef> into a RefCell<Vec<ExtensionRef>>, and making a sort of iterator that looks like this:

pub struct ExtensionListIterator<'l>
{
	list: Ref<'l, Vec<ExtensionRef>>,
	extension_refs: Vec<Ref<'l, Extension>>,
}

The idea is to get a Ref to the underlying list, get Refs to all the extensions, and have the struct acts as a handle to all of these. While the struct is alive, the list is locked and all extensions are pre-borrowed. However, I'm struggling a bit with the syntax, especially the line which constructs a Vec<Ref<'l, Extension>>:

impl ExtensionList
{
	pub fn get_iterator<'l>(&'l self) -> Result<ExtensionListIterator<'l>, BorrowError>
	{
		let list_ref: Ref<'l, Vec<ExtensionRef>> = self.extensions.try_borrow()?;
		// This definitely isn't correct!
		let extension_refs: Vec<Ref<'l, Extension>> = list_ref.iter().map(|ext_ref| ext_ref.get_extension()).collect()?;
		return Ok(ExtensionListIterator { list: list_ref, extension_refs });
	}
}

Is this the best way to be going about this, or is there a more ergonomic way to achieve what I want?

2

the thing you need to ask yourself, that will define what might be the best way to solve this, is why do you need those refcells.

try_borrow and try_borrow_mut can fail because the values may be borrowed by something else at the same time. so you ideally want to minimize that borrowing to avoid the conflicts.

but your ExtensionListIterator does the opposite of that by forcibly borrowing every thing at the same time.

3

Maybe reduce the OO style and solve the problem more directly.

For a single method/function to return a collection of some items, there is barely any justification for a own type, just wrap a Vec.

This looks like just a tuple: some values belong together.

There shouldn't be any access or storage policy imposed on the caller. It's perfectly fine to choose a struct with named fields instead of an anonymous tuple. But make the fields pub and drop the RefCell.

Additionally, maybe make the name more obvious:

pub struct NamedExtension
{
	pub name: String,
	pub extension: Extension,
}

And the directory scanning function becomes something like this:

pub fn load_extensions(directory: &Path) -> Result<Vec<NamedExtension>, std::io::Error> {
   ...
}

No shallow OO like wrappers that add only burden.